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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D72018#1801725 <https://reviews.llvm.org/D72018#1801725>, @aaron.ballman wrote:

> 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?


This is a valid concern. Currently, this is a common practice in the static analyzer to exclude certain functions from the checks, but until now, we usually hard coded the name/signature of the function rather than relying on an attribute. This did make sense, since it is an implementation detail that belongs to the analyzer. The main reason why we was thinking about making this an annotation, because if there is an annotation, when the API changes, we do not need to update the analyzer, and do not need to make sure that the analyzer version and the API matches.

> 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.

This is correct. If we want to diagnose non-existent checker names we definitely would need a diagnostic flag and probably we want to have the check off by default. The reason other reason (apart from backward compatibility) is that, some users might load non-upstreamed checkers as plugins for some builds (but not for others).

I think if there are many problems with this concept, I will fall back to hard code this information in the checker instead of using an annotation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72018