[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D72018#1801450 <https://reviews.llvm.org/D72018#1801450>, @xazax.hun wrote:

> In D72018#1800018 <https://reviews.llvm.org/D72018#1800018>, @aaron.ballman wrote:
>
> > This seems like a very specialized attribute for the static analyzer and I'm not certain how much utility it adds, but that may be because I don't understand the analyzer requirements sufficiently yet. It seems as though this attribute is intended to suppress diagnostics within a certain function for a certain check, much like `gsl::suppress` does for the C++ Core Guidelines. Is that correct? If so, perhaps we should just reuse that attribute (but with a different spelling)?
>
>
> In some sense they are similar, but not entirely. When I annotate a function with `gsl::suppress`, I would expect to suppress diagnostics inside the function. When I annotate a function with `analyzer_checker_ignore`, that would suppress diagnostics in the callers of the function. So while the `gsl::suppress` is merely a way to suppress diagnostics, this attribute does change how the analysis is carried out.


Thank you for the explanation, that makes sense, but I'm still a bit uncomfortable. In this case, it seems like the author of the API needs to 1) know that users will be analyzing the code with clang's static analyzer, 2) and that a particular function will cause problems for a specific check. It seems like the API author won't be in a position to add the attribute to the correct APIs, and what we really need is something for a *user* to write on an existing declaration they may not control or have the ability to modify the declaration of. I am not certain how much I like this idea, but perhaps we could allow the attribute to be written on a redeclaration and apply it to the canonical declaration so that users could add the attribute instead of relying on the library author to do it for them?

Also, does this mean that changing the name of a check in the static analyzer will now have the potential to break code? e.g., if a check move from the alpha group to the core group, the check name changes, and thus code needs to be updated. I presume one of the things this attribute will do is diagnose when an unknown check name is given? This has the potential to introduce diagnostics where a library is written for a newer clang but being compiled within an older clang, so we'd probably need a warning flag or something else to control the diagnostic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018