Bug 410751 - [check proposal] not emitting signals in noexcept function
Summary: [check proposal] not emitting signals in noexcept function
Status: REPORTED
Alias: None
Product: clazy
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other All
: NOR wishlist
Target Milestone: ---
Assignee: Unassigned bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-09 08:33 UTC by Jean-Michaël Celerier
Modified: 2019-09-01 13:32 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Michaël Celerier 2019-08-09 08:33:25 UTC
SUMMARY

This is a proposal for a new check ; for every method, the following assertion must hold :  
- if the method emits a signal, then the method must not be marked noexcept (or conversely, if a method is marked noexcept, it must not emit signals). 

The rationale is that while your class may not throw any exceptions, maybe the user of your class does and expects to be able to catch them at the top of the event loop, e.g. simply to have a catch-all to show an error dialog to the user.

But if you mark e.g. a setter for a property noexcept, it means that all the persons who will connect to your propertyChanged signal must also ensure that no exceptions are thrown from their side, else they get std::terminate.

The check could be relaxed if the slot of the signal is also marked noexcept.

I don't think that we should aim to make the check recursive since this requires whole-program analysis, but just having it run on the main emitters of signals (generally setFoo(...) functions) would already go a long way towards preventing crashes due to over-zealous noexcept refactorings.
Comment 1 Sergio Martins 2019-08-09 12:52:46 UTC
>> The check could be relaxed if the slot of the signal is also marked noexcept.

Maybe not, as signals don't know (or shouldn't know) which slots are connected to them. So I would make this check look at all methods.

In terms of implementation, catch all CallExpr, check if it's a signal (there's helpers for that already), check if method is noexpr.