Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Some of the code is copied from UnhandledExceptionAtNewCheck. Probably these checks can be merged into one NewExceptionHandlingCheck?
Thanks for this new check. Could you please link here results of this checker on som relevant open source projects?
I'm really not sure this is the right approach to solve the problem. If the concern here is leaking on exceptions, then the goalpost should be flagging all calls to global new and recommend a smart pointer factory instead. I think that check could even be made as a cppcoreguideline check
CppCoreGuidelines rule r13-perform-at-most-one-explicit-resource-allocation-in-a-single-expression-statement is applicable for this problem. The current check is more restricted. If the check is moved to cppcoreguidelines it should check for any multiple new in a single expression and probably for other "resource allocation" that is not new (if there is such case at all that can not be changed to a safe function call or is not deprecated by other rules). Probably we can have an option to switch the behavior (any 2 new or the current way) and move the check to cppcoreguidelines.
This check is not about removing any use of new. It is only about the memory leak problem that is easy to detect and mentioned in the CppCoreGuidelines and CERT MEM52-CPP rules. If it is moved to the cppcoreguidelines module I would add an option that makes the check work like it works in this patch now, if it is turned off the check can find any expression where multiple new occurs (even when || and similar operators are used). My question is would the reviewers accept the check in the current form or should it be moved to cppcoreguidelines?