[PATCH] D36475: [analyzer] Add "create_sink" annotation support to MagentaHandleChecker

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 5 10:31:19 PDT 2017


dcoughlin added a comment.

I'm a bit surprised that this is needed for unit tests. Aren't unit tests supposed to clean up their own state? Leaking a scarce resource in one unit test can cause another unit test to fail, which can be hard to track down. Or is this for deliberately testing a scenario where a resource is leaked? If you're deliberately testing a leaking scenario, it seems to me that using #ifndef __clang_analyzer__ would probably be an easier suppression mechanism for the test author  to understand and maintain.

Can you elaborate on why you're running the analyzer on units tests and how reports from the analyzer on unit tests are used? I think it would be helpful for us to understand your work flow.

Also, in general, my feeling is the exposing the internal mechanisms of the analyzer (here, the notion of a sink) in an attribute is not good for users. Ideally, attributes should be used to describe the behavior of code that they annotate. This means that they also serve as documentation for the user and not just as an ad hoc mechanism to communicate with the analyzer.


https://reviews.llvm.org/D36475





More information about the cfe-commits mailing list