On Mon Jun 15, 2026 at 4:36 PM WEST, David Malcolm wrote:
> On Sun, 2026-06-14 at 19:41 +0100, Egas Ribeiro wrote:
>
> [CCing the C++ frontend maintainers, for their input]
>
> Hi Egas.  Thanks for the patch.  Various comments inline below...
>
>> Thoughts on adding lang_hooks.exception_handler_matches_p next to the
>> other EH hooks? Or would it be better to keep analyzer-specific
>> semantics out of the general langhook table?
>
> The approach you've got in this patch looks simple, which is good -
> assuming that the C++ FE maintainers are OK with exposing this as a
> langhook.  One issue we might eventually run into is the LTO case where
> the analyzer might be analyzing TUs from multiple languages (e.g. a
> mixture of C++ and C).  But given that the analyzer barely supports LTO
> right now, I think going with the simple approach for now is OK.
>

Yes, with LTO this won't work out. In that case, the original
language_semantics class would be interesting since it would allow
multiple instances of the hook. I wanted to keep the approach as simple
as possible but I did not consider LTO when I went with this. I'll go
with whatever you prefer, we can keep the language hook if you are OK
with that.

>> 
>> I left the ChangeLog entries and the commit message minimal for this
>> RFC, if this direction is OK I will submit it again with a complete
>> commit message.
>
> I think the ChangeLog entries are the right level of verbosity, but
> please do a 2nd iteration of the patch with a proper commit message.
>

Will do. I'll include that along with testing if we move forward with
this approach.

>> exception-subclass-1's xfail now passes with the fix as expected,
>> however exception-subclass-2 still fails; I'll ask about this
>> separately
>> in this thread.
>
> I'll take a look there.
>

>> +void test_object_vs_pointer ()
>> +{
>> +  try {
>> +    throw Derived ();
>> +  }
>> +  catch (Base *) {
>> +    __analyzer_dump_path (); // { dg-bogus "path" }
>> +  }
>> +}
>
> Out of curiosity, does g++ issue any kind of warning for this?  I
> wonder if it should.
>
It doesn't emit any warnings right now with -Wall -Wextra -Wexceptions
it seems (clang doesn't either fwiw).
>
> This looks good for a first iteration, but let's have some input from a
> C++ frontend reviewer (e.g. Jason).
>
> What level of testing has the patch had?
>

I haven't ran full tests yet (I only ran exception related tests). I
felt it would be more productive to first discuss the general approach
before I ran some tests (I usually test the whole patch first but here I
wanted to know the intended direction before spending the time polishing
the details). 

Next time I can submit a mergeable patch right away, but I couldn't
decide on a direction thus wanted to open up the discussion to start of
the project.

Thanks,
Egas

Reply via email to