Re: [PATCH] gdb/testsuite: allow "require" callbacks to provide a reason

[Simon Marchi <simon.marchi@efficios.com>]

On 3/28/23 11:44, Tom de Vries wrote:
> On 3/28/23 14:23, Simon Marchi via Gdb-patches wrote:
>> When an allow_* proc returns false, it can be a bit difficult what check
>> failed exactly, if the procedure does multiple checks.  To make
>> investigation easier, I propose to allow the "require" callbacks to be
>> able to return a list of two elements: the zero/non-zero value, and a
>> reason string.
>>
> 
> Hi,
> 
> I like the idea.
> 
> The only question I had was what the desired behaviour is for:
> ...
>   return {1 "bla"}
> ...
> 
> AFAICT, the current implementation just ignores "bla", and I wondered if perhaps we should error out to avoid the impression that something will be done with "bla".
> 
> That however will make this fail if $res == 1:
> ...
> set res [try_foo]
> return {$res "foo"}
> ...
> so probably that's just too restrictive.

I can't really think of a use case where we would return 1, with a
justification, if we always formulate our procs in the "allow" form.
That is:

  proc allow_foo_tests { } {
    if ... {
      return {0 "reason A"}
    }

    if ... {
      return {0 "reason B"}
    }

    return 1
  }

But if we have a proc in the opposite "prevent" or "skip" form (like we
had before the conversion to "allow"), it would typically written as:

  proc prevent_foo_tests { } {
    if ... {
      return {1 "reason A"}
    }

    if ... {
      return {1 "reason B"}
    }

    return 0
  }

and it could be used as:

  require !prevent_foo_tests

So that's an example where we could theoritically return 1 and use the
reason string.  It just won't happen often, because we it's better to
write things in the "allow" form, to avoid the double negative.


> LGTM as-is.

Thanks, will push.

Simon