On 11-06-2020 18:56, Pedro Alves wrote:
> On 6/11/20 5:25 PM, Tom de Vries wrote:
>> On 11-06-2020 17:31, Pedro Alves wrote:
> 
>>> I'd think it would be cleaner to override unknown in gdb_init,
>>> and restore in gdb_finish.  No need for filename matching that way.
>>> Like below.  Any reason you didn't go for this instead?
>>>
>> I don't think it's cleaner, because we run default_gdb_init, as well the
>> bit of proc runtest between ${tool}_init a ${tool}_finish with the
>> altered ::unknown.
> 
> Well, that is code that is exercised by all testscases, it's part of the
> framework.  It's not going to ever contain calls to procedures that
> don't exist that go unnoticed for long.  The whole testsuite would
> collapse.  We can move the overriding until after default_gdb_init
> is called, even, so that even less code runs under gdb's "unknown":
> 
>     set res [default_gdb_init $test_file_name]
> 
>     # Skip dejagnu_unknown while running a testcase to prevent it from
>     # exiting and aborting the entire test run.  Save the unknown proc
>     # defined by DejaGnu, and override the unknown proc with a
>     # gdb-local version.
>     rename ::unknown ::dejagnu_unknown
>     proc unknown { args } {
> 	return [uplevel 1 ::tcl_unknown $args]
>     }
> 
>     return $res
>   }
> 
>> Alternatively, we could override source to detect the precise point that
>> runtest hands control to the test-case, as attached.  But there's still
>> filename matching.  [ Note btw, that this approach changes the scope of
>> the fix slightly. ]
> 
> Overriding standard procs and filename matching strikes me as uglier than
> using the hooks that dejagnu provides (init/finish), but I can also see your
> point of only overriding unknown for the exact duration of the sourced file
> being cleaner.  Thus, I'm not objecting and leave it up to you.  I agree
> that it's nice that we don't abort the whole testsuite.
> 

I'm convinced now, it's best to use dejagnu hooks (although my questions
related to gdb_init/default_gdb_init remain).

> BTW, isn't "tcl_unknown" an internal DejaGnu name?  We should document
> that in a comment, since this could break if DejaGnu changes the renamed
> proc's name.

Done.  Committed as attached.

Thanks,
- Tom


>  Another reason to ask DejaGnu to handle this for us cleanly.
>