Bug 410751

Summary: [check proposal] not emitting signals in noexcept function
Product: [Developer tools] clazy Reporter: Jean-Michaël Celerier <jeanmichael.celerier>
Component: generalAssignee: Unassigned bugs <unassigned-bugs-null>
Status: REPORTED ---    
Severity: wishlist CC: smartins
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: All   
Latest Commit: Version Fixed In:
Sentry Crash Report:

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.