I’m trying to land Valeriy’s old patch: ⚙ D93110 [analyzer] Implement fine-grained suppressions via attributes so I’m curious what do you all think of it! And of potential future directions. The patch introduces an attribute [[clang::suppress(...)]] to suppress arbitrary static analysis false positives.
It’s like #ifndef __clang_analyzer__ but not ugly. It’s like // NOLINT but first-class citizen. It’s like [[gsl::suppress]] but universal. It’s like Rust #[allow(...)] but in C and C++.
Because it’s a statement attribute, you can put it on arbitrary individual statements, or on a compound statement, or on a control flow construct, or even on a NullStmt if you like. It also respects uniquing locations, so you can suppress leak warnings by annotating either the leak point or the allocation site.
The patch adds support for the attribute in the static analyzer but there’s no fundamental reason we can’t teach clang-tidy to use it; and I’ll probably attempt to do that in a follow-up patch. Initially we also considered teaching it about clang frontend warnings, but I no longer think it’s a good idea; the problem is that it won’t be able to suppress warnings that are emitted before the attribute object is constructed and the attributed statement is fully parsed, which is just inconsistent user experience. So with frontend warnings it’s probably better to stick to #pragma clang diagnostic.
Conversely, it may be a good idea to introduce this attribute “in pragma form” in order to suppress warnings in ranges that span multiple functions. (I don’t think #pragma clang attribute is going to give us that for free; it probably doesn’t work well with statement attributes, so this’ll need some manual work.)
Related to that, currently the attribute can’t be placed on functions or classes, because back when the patch was written we hadn’t made up our mind whether it should suppress warnings in things outside of its immediate scope, such as redeclarations of the function, or out-of-line methods of the class. These days we’re thinking that it should stick to the lexical scope, potentially with an alternative spelling to auto-propagate more aggressively. This feature is quite desirable for AST-based checks that may emit warnings against non-statements, esp. considering the clang-tidy use case where such checkers are much more popular.
Finally, the elephant in the room is checker names. For now the attribute doesn’t allow you to specify which classes of warnings are suppressed. I just don’t like our static analyzer checker names; they aren’t great, separations into packages is arbitrary and a lot of checkers are misplaced, checkers that emit warnings from inside core should have never been placed there. We want to fix checker names, but this means that we can’t promise that they’ll be stable, so we don’t want people to use them.
In the past I have floated an idea to replace checker names with a system of “hashtags” (“keywords”) that each individual diagnostic is described with, so that diagnostics could be turned on or off based on these hashtags. Multiple hashtags can be attached to the same diagnostic, which means we have little incentive to break them.
While this won’t happen overnight, I think for the purposes of the [[clang::suppress]] mechanism we could make the first step in this direction: introduce a notion of the “warning hashtag” and make the suppression attribute react to the hashtag attached to the warning. We could automatically promote existing checker names as well as their respective package names to hashtags (both in the static analyzer, eg. [#unix #unix.Malloc], and in clang-tidy, eg. [#bugprone #bugprone-assert-side-effect]) so that they could be used for suppression if the user is already familiar with them, but in the static analyzer we don’t want people to rely on them being stable, so in any case we’ll continue to incrementally introduce more and more “better” hashtags to cover specific checkers.
