Re: [PATCH v2] Logical short circuiting with argument lists

[Tom Tromey <tom at tromey dot com>]

>>>>> "Richard" == Richard Bunt <richard.bunt@arm.com> writes:

Thank you for the patch.

Richard> koenig.exp was updated to include a testcase which exercises the
Richard> modified skip logic in OP_VAR_VALUE, as it falls through from
Richard> OP_ADL_FUNC.

Nice, thanks.

Richard> +void
Richard> +skip_undetermined_arglist (int nargs, struct expression *exp, int *pos,
Richard> +			   enum noside noside)

This needs an introductory comment.

Richard> @@ -1283,15 +1294,20 @@ evaluate_subexp_standard (struct type *expect_type,
Richard>      case OP_ADL_FUNC:
Richard>      case OP_VAR_VALUE:
Richard>        (*pos) += 3;
Richard> -      if (noside == EVAL_SKIP)
Richard> -	return eval_skip_value (exp);
Richard> -
Richard>        {
Richard>  	symbol *var = exp->elts[pc + 2].symbol;
Richard>  	if (TYPE_CODE (SYMBOL_TYPE (var)) == TYPE_CODE_ERROR)
Richard>  	  error_unknown_type (SYMBOL_PRINT_NAME (var));

With this change I think it would read more nicely if you sunk the
"(*pos) += 3;" into the block.

I wonder if we really want this to error in the case where the symbol
isn't found.  But I suppose it is necessary for your patch to work.

I tend to think it is fine; I couldn't think of a plausible,
non-pathological way that I would want to use the non-erroring behavior.

Though, another option is to do this in a fortran-specific way.

Richard> +if ![runto [gdb_get_line_number "post_truth_table_init"] ] then {

I usually brace the conditions and cuddle the brackets, so

    if {!runto [gdb...]]} then {

The bracing is super Tcl nerdery.

Richard> +# Vary conditional and input over the standard truth table.
Richard> +# Test that the debugger can evaluate expressions of the form
Richard> +# a(x,y) .OR./.AND. a(a,b) correctly.
Richard> +foreach_with_prefix truth_table_index {1 2 3 4}  {
Richard> +    gdb_test "p truth_table($truth_table_index, 1) \
Richard> +	     .OR. truth_table($truth_table_index, 2)" \

I think it's better to just have a longer line here than to split a
string in the middle.

Richard> +    set argument_list "$argument_list .TRUE.,"

You can write this as

    append argument_list " .TRUE.,"

Richard> +	gdb_test "p .TRUE. .OR. binary_string($range1) .OR. truth_table($range2, 1)" \

For some of the tests -- I'm afraid I didn't enumerate them -- you will
need to supply a different (and unique) test name to gdb_test, because
there is a rule against tests having parenthesized text at the end of
their name:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Tom