[PATCH] D124462: [Analyzer] Fix clang::ento::taint::dumpTaint definition

Marco Antognini via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 28 07:06:01 PDT 2022


mantognini added a comment.

In D124462#3477115 <https://reviews.llvm.org/D124462#3477115>, @steakhal wrote:

> Although gcc (and probably clang too) allows specifying `__attribute__((noinline))` at any declaration (by merging //compatible// attributes), I would prefer not to repeat ourselves.
> The attribute must be present at the header to force all usages not to inline it, hence I would rather drop such attributes from the definition files.

On further inspection, I agree with you and will update the patch (+ description).

A few things misled me. First, attributes are complex, with standard and compiler-specific ones having different requirements. Then, Clang's doc <https://clang.llvm.org/docs/AttributeReference.html#used> about `used` talks about definitions, not declarations. GCC is clearer and always talks about declarations. Fortunately, https://godbolt.org/z/PnW7x9Ezz shows Clang inherits, as expected, the attributes of interest here.

But I was mainly misled because, specifically for `LLVM_DUMP_METHOD`, the trend in LLVM seems to apply it to definitions:

  $ git grep -Ee '::dump\(\) const \{.*' | grep -ve LLVM_DUMP_METHOD | wc -l
  92
  $ git grep -Ee '::dump\(\) const \{.*' | grep -e LLVM_DUMP_METHOD | wc -l
  184

Anyway, thanks for your feedback -- this fairly trivial patch turned out to reveal something interesting to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124462



More information about the cfe-commits mailing list