[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

Fri Feb 12 08:36:38 PST 2021

vsavchenko added a comment.

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

> In D93110#2529917 <https://reviews.llvm.org/D93110#2529917>, @NoQ wrote:
>
>>> What I want to avoid is having a static analyzer-specific suppression attribute, and a different one for clang-tidy, and a third one for the frontend.
>>
>> Given that there are already multiple suppression mechanisms circulating around (clang-tidy's `// NOLINT`, frontend pragmas, static analyzer's `#ifdef __clang_analyzer__`, now also attribute), i guess a path forward would be to eventually add support for multiple mechanisms everywhere.  This may be tedious to implement but benefits may be well worth it. Consistency across tools is important when tools are used together; for instance, when clang-tidy integrates static analyzer, static analyzer automatically starts supporting `// NOLINT` through clang-tidy's diagnostic consumer magic. This would also be amazing for discoverability: users can keep using their preferred mechanism and it'd just work out of the box when they add a new tool to their arsenal.
>
> To be clear, I'm fine with multiple mechanisms, but not fine with multiple of the same mechanisms. e.g., I don't think it's an issue that we can suppress via the command line, pragmas, NOLINT comments, and attribute usage, but I do think it's a problem if there's one attribute used by clang-tidy and a different attribute used by the static analyzer, and a third attribute for clang itself. Using different attributes causes users to have to consider what basically amount to implementation details of their toolchain and makes it harder to do things like integrate clang-tidy and the clang static analyzer together.

We can integrate these solutions together for clang-tidy and for clang static analyzer, it's not a problem at all.  I would've used the existing Suppress attribute if it was without `gsl` part.  As I mentioned in the summary, at the moment, we want to use it ONLY inside functions.  I can see that `SuppressAttr` is not used anywhere in the whole, but I can't be sure that it's not used somewhere externally.  So, I couldn't just reduce the scope of it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93110