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

[sammccall]

Summary

Certain idioms are ignored by -Wunused in header files only.
The current "is a header" check assumes that if headers are the main file, we're
building a PCH or a module or something. However in tools we may be parsing the
header in its own right, but still want to treat it as a header.

Fixes https://github.com/clangd/vscode-clangd/issues/360

[sammccall]

add bug link to description

[sammccall]

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.

[kadircet]

clang/lib/Sema/SemaDecl.cpp

1784

i think we can just bail out early here when the main file is a header file (i.e. lang opts have IsHeaderFile set).

[sammccall]

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

clang/lib/Sema/SemaDecl.cpp

1784

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)

[kadircet]

clang/lib/Sema/SemaDecl.cpp

1784

SG. i was actually worried about the behaviour implied by the name "isMainFileLoc". i feel like both TU_Complete and IsHeaderFile check should actually happen here (or at least outside of the isMainFileLoc).

[nridge]

nit: typo in commit message ("hedaer")

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

::testing::Matcher<const Diag &> withFix(::testing::Matcher<Fix> FixMatcher) {
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
 }
 
 ::testing::Matcher<const Diag &> withFix(::testing::Matcher<Fix> FixMatcher1,
                                          ::testing::Matcher<Fix> FixMatcher2) {
   return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
 }
 
+::testing::Matcher<const Diag &> withID(unsigned ID) {
+  return Field(&Diag::ID, ID);
+}
 ::testing::Matcher<const Diag &>
 withNote(::testing::Matcher<Note> NoteMatcher) {
   return Field(&Diag::Notes, ElementsAre(NoteMatcher));
 }
 
 ::testing::Matcher<const Diag &>
 withNote(::testing::Matcher<Note> NoteMatcher1,
          ::testing::Matcher<Note> NoteMatcher2) {
EXPECT_THAT(
       *TU.build().getDiagnostics(),
       UnorderedElementsAre(AllOf(
           Diag(Source.range("diag"), "no matching function for call to 'foo'"),
           withFix(Fix(Source.range("fix"), "&",
                       "candidate function not viable: no known conversion from "
                       "'int' to 'int *' for 1st argument; take the address of "
                       "the argument with &")))));
 }
+
+TEST(DiagnosticsTest, UnusedInHeader) {
+  // Clang diagnoses unused static inline functions outside headers.
+  auto TU = TestTU::withCode("static inline void foo(void) {}");
+  TU.ExtraArgs.push_back("-Wunused-function");
+  TU.Filename = "test.c";
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+              ElementsAre(withID(diag::warn_unused_function)));
+  // Sema should recognize a *.h file open in clangd as a header.
+  // https://github.com/clangd/vscode-clangd/issues/360
+  TU.Filename = "test.h";
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

clang/lib/Sema/SemaDecl.cpp

bool Sema::mightHaveNonExternalLinkage(const DeclaratorDecl *D) {
   }
 
   return !D->isExternallyVisible();
 }
 
 // FIXME: This needs to be refactored; some other isInMainFile users want
 // these semantics.
 static bool isMainFileLoc(const Sema &S, SourceLocation Loc) {
-  if (S.TUKind != TU_Complete)
+  if (S.TUKind != TU_Complete || S.getLangOpts().IsHeaderFile)
     return false;
   return S.SourceMgr.isInMainFile(Loc);
 }
 
 bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
   assert(D);
 
   if (D->isInvalidDecl() || D->isUsed() || D->hasAttr<UnusedAttr>())
     return false;
 
   // Ignore all entities declared within templates, and out-of-line definitions
   // of members of class templates.
   if (D->getDeclContext()->isDependentContext() ||
       D->getLexicalDeclContext()->isDependentContext())
     return false;