[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

Thu Jul 14 02:44:13 PDT 2022

sammccall added a comment.

In D129642#3648197 <https://reviews.llvm.org/D129642#3648197>, @sammccall wrote:

> It's possible we should *only* be checking the IsHeaderFile flag here and drop all the sourcemanager stuff, but I'm not sure of the implications - thoughts appreciated.

As discussed offline this idea doesn't make sense. LangOpts tells us about the main file, so we still need to check that it's in the main file for that to be relevant.

================
Comment at: clang/lib/Sema/SemaDecl.cpp:1784

   if (D->isInvalidDecl() || D->isUsed() || D->hasAttr<UnusedAttr>())
     return false;
----------------
kadircet wrote:
> i think we can just bail out early here when the main file is a header file (i.e. lang opts have `IsHeaderFile` set).
This makes sense.

I'd rather do this as a separate commit in case we need to revert.
This one is a functional change the IsHeaderFile check to a place that's already testing for header-like contexts.
The other would be a (hopefully) NFC for performance.

(This function isn't terribly hot in the clangd profiles I have, but seems like a reasonable change to me anyway)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129642