This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Logical short circuiting with argument lists
- From: Tom Tromey <tom at tromey dot com>
- To: Richard Bunt <richard dot bunt at arm dot com>
- Cc: gdb-patches at sourceware dot org, nd at arm dot com
- Date: Fri, 07 Sep 2018 16:23:25 -0600
- Subject: Re: [PATCH v2] Logical short circuiting with argument lists
- References: <79494820-f795-ad3c-8546-0ad288cd05b0@arm.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