[PCH] make sure to not warn about unused macros from -D

[llunak]

Summary

If a PCH is used for compilation, SourceManager::isInMainFile() returns true even for the "<built-in>" predefines area. Using -D only for the TU compilation may trigger -Wunused-macros for it. It is admitedly a bit fishy to set a macro only for a TU and not for the PCH, but this works fine if the PCH does not use the macro (I couldn't find a statement on this for Clang, but GCC explicitly allows this in the docs).

[llunak]

Ping.

[llunak]

Ping..

[llunak]

Ping...

[llunak]

Ping....

[dblaikie]

Looks pretty reasonable to me - but I don't know enough about the macro and source location infrastructure. @rsmith - mind taking a quick look?

[llunak]

Ping.....

[llunak]

Ping......

[rsmith]

Looks OK as a workaround. Do you know why we consider these to be in the main file? If we could fix that in the source manager, that'd seem preferable.

[llunak]

In D73846#1991330, @rsmith wrote:

Looks OK as a workaround. Do you know why we consider these to be in the main file? If we could fix that in the source manager, that'd seem preferable.

According to my testing, SourceManager::isInMainFile() can handle "<built-in>" locations in 3 ways:

• for macros defined using -D on the command line the control flow returns false in the "if (FI.hasLineDirectives())" block
• for built-in macros such as clang the control flow enters the same "if (FI.hasLineDirectives())" block, but Entry->IncludeOffset is 0, so the flow then reaches the final "return FI.getIncludeLoc().isInvalid();", which returns true
• if PCH is used, macros defined using -D on the command line do not even enter "if (FI.hasLineDirectives())" and so they end up returning true the same way built-in macros do

But I don't understand this enough to know why and what that actually means.

I've also tried a patch that added SourceManager::setPredefinesFileID() and moved the check from this patch to SourceManager::isInMainFile(), but then tests fail because apparently Preprocessor::setPredefinesFileID() may be called multiple times.

clang/lib/Lex/PPDirectives.cpp

void Preprocessor::HandleDefineDirective(
   DefMacroDirective *MD =
       appendDefMacroDirective(MacroNameTok.getIdentifierInfo(), MI);
 
   assert(!MI->isUsed());
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
       !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
-      !MacroExpansionInDirectivesOverride) {
+      !MacroExpansionInDirectivesOverride &&
+      getSourceManager().getFileID(MI->getDefinitionLoc()) !=
+          getPredefinesFileID()) {
     MI->setIsWarnIfUnused(true);
     WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
 
   // If the callbacks want to know, tell them about the macro definition.
   if (Callbacks)
     Callbacks->MacroDefined(MacroNameTok, MD);
 }

clang/test/PCH/cli-macro.c

+// Test this without pch.
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -Wunused-macros -emit-pch -o %t %s
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// -Dunused=1 is intentionally not set for the pch.
+// There still should be no unused warning for a macro from the command line.
+