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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to