https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/177078
>From 90dfe27db168da7f160cacfa65fc2c583b1cbce0 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Mon, 5 Jan 2026 14:41:22 -0800 Subject: [PATCH] [clang][Modules] Fix unexpected warnings triggered by a PCH and a module with config macros (#174034) When a PCH is compiled with macro definitions on the command line, such as `-DCONFIG1`, an unexpected warning can occur if the macro definitions happen to belong to an imported module's config macros. The warning may look like the following: ``` definition of configuration macro 'CONFIG1' has no effect on the import of 'Mod1'; pass '-DCONFIG1=...' on the command line to configure the module ``` while `-DCONFIG1` is clearly on the command line when `clang` compiles the source that uses the PCH and the module. The reason this can happen is a combination of two things: 1. The logic that checks for config macros is not aware of any command line macros passed through the PCH ([here](https://github.com/llvm/llvm-project/blob/7976ac990000a58a7474269a3ca95e16aed8c35b/clang/lib/Frontend/CompilerInstance.cpp#L1562)). 2. `clang` _replaces_ the predefined macros on the command line with the predefined macros from the PCH, which does not include any builtins ([here](https://github.com/llvm/llvm-project/blob/7976ac990000a58a7474269a3ca95e16aed8c35b/clang/lib/Frontend/CompilerInstance.cpp#L679)). This PR teaches the preprocessor to recognize the command line macro definitions passed transitively through the PCH, so that the error check does not miss these definitions by mistake. The config macro itself works fine, and it is only the error check that needs fixing. rdar://95261458 (cherry picked from commit 71925cbdf80d4fc88ef844e726f89aa71c64bceb) --- clang/include/clang/Lex/Preprocessor.h | 11 +++++ clang/lib/Frontend/CompilerInstance.cpp | 4 +- clang/lib/Lex/PPMacroExpansion.cpp | 29 +++++++++++ .../Inputs/pch-config-macros/include/Mod1.h | 5 ++ .../include/module.modulemap | 4 ++ clang/test/Modules/pch-config-macros.c | 48 +++++++++++++++++++ 6 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 clang/test/Modules/Inputs/pch-config-macros/include/Mod1.h create mode 100644 clang/test/Modules/Inputs/pch-config-macros/include/module.modulemap create mode 100644 clang/test/Modules/pch-config-macros.c diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 5fb83eafc6b2a..1845a47dba0f3 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -1180,6 +1180,10 @@ class Preprocessor { /// The file ID for the PCH through header. FileID PCHThroughHeaderFileID; + /// The file ID for the predefines that come from the PCH. + /// This is only set when modules are in effect. + FileID PCHPredefinesFileID; + /// Whether tokens are being skipped until a #pragma hdrstop is seen. bool SkippingUntilPragmaHdrStop = false; @@ -1379,6 +1383,13 @@ class Preprocessor { /// Returns the FileID for the preprocessor predefines. FileID getPredefinesFileID() const { return PredefinesFileID; } + /// Returns the FileID for the predefines loaded from the PCH. + FileID getPCHPredefinesFileID() const { + assert(getLangOpts().Modules && + "PCHPredefinedFileID is only set when modules is in effect!"); + return PCHPredefinesFileID; + } + /// \{ /// Accessors for preprocessor callbacks. /// diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index cb3e6fb9688a9..2498eecc50cd6 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1583,7 +1583,9 @@ static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro, for (auto *MD = LatestLocalMD; MD; MD = MD->getPrevious()) { // We only care about the predefines buffer. FileID FID = SourceMgr.getFileID(MD->getLocation()); - if (FID.isInvalid() || FID != PP.getPredefinesFileID()) + if (FID.isInvalid()) + continue; + if (FID != PP.getPredefinesFileID() && FID != PP.getPCHPredefinesFileID()) continue; if (auto *DMD = dyn_cast<DefMacroDirective>(MD)) CmdLineDefinition = DMD->getMacroInfo(); diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 20b8fe585a007..0e92398184fb0 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -125,6 +125,35 @@ void Preprocessor::setLoadedMacroDirective(IdentifierInfo *II, II->setHasMacroDefinition(true); if (!MD->isDefined() && !LeafModuleMacros.contains(II)) II->setHasMacroDefinition(false); + + if (getLangOpts().Modules) { + // When both modules and a PCH are used, we may run into the following + // situation: + // - the PCH is compiled with macro definitions on the command line. + // - the modules are compiled with the same set of macros on the command + // line. + // In this case, clang needs to know that some predefined macros exist + // over the command line transitively through the PCH and some are passed + // directly over the command line. The preprocessor stores + // PCHPredefinesFileID so later it is aware of macros defined transitively + // through the PCH's compilation. + auto MDLoc = MD->getLocation(); + + if (SourceMgr.isWrittenInCommandLineFile(MDLoc)) { + auto MDFileID = SourceMgr.getFileID(MDLoc); + if (PCHPredefinesFileID.isInvalid()) + PCHPredefinesFileID = MDFileID; + else { + // The PCH and all the chain of headers it includes must be + // compiled with the exact same set of macros defined over the + // command line. No different macros should be defined over + // different command line invocations. This means that all the macros' + // source locations should have the same MDFileID. + assert(MDFileID == PCHPredefinesFileID && + "PCHBuiltinFileID must be consistent!"); + } + } + } } ModuleMacro *Preprocessor::addModuleMacro(Module *Mod, IdentifierInfo *II, diff --git a/clang/test/Modules/Inputs/pch-config-macros/include/Mod1.h b/clang/test/Modules/Inputs/pch-config-macros/include/Mod1.h new file mode 100644 index 0000000000000..3b8f33877dcd2 --- /dev/null +++ b/clang/test/Modules/Inputs/pch-config-macros/include/Mod1.h @@ -0,0 +1,5 @@ +#if CONFIG1 +int foo() { return 42; } +#else +int foo() { return 43; } +#endif diff --git a/clang/test/Modules/Inputs/pch-config-macros/include/module.modulemap b/clang/test/Modules/Inputs/pch-config-macros/include/module.modulemap new file mode 100644 index 0000000000000..9d5f2e714ec88 --- /dev/null +++ b/clang/test/Modules/Inputs/pch-config-macros/include/module.modulemap @@ -0,0 +1,4 @@ +module Mod1 { + header "Mod1.h" + config_macros CONFIG1,CONFIG2 +} diff --git a/clang/test/Modules/pch-config-macros.c b/clang/test/Modules/pch-config-macros.c new file mode 100644 index 0000000000000..67a63dbe1286e --- /dev/null +++ b/clang/test/Modules/pch-config-macros.c @@ -0,0 +1,48 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: cd %t + +// This test builds two PCHs. bridging.h.pch depends on h1.h.pch. +// Then the test uses bridiging.h.pch in a source file that imports +// a module with config macros. +// This is a normal use case and no warnings should be issued. +// RUN: %clang_cc1 -fmodules \ +// RUN: -fmodule-map-file=%S/Inputs/pch-config-macros/include/module.modulemap \ +// RUN: -fmodules-cache-path=%t/cache -I %S/Inputs/pch-config-macros/include \ +// RUN: h1.h -emit-pch -o h1.h.pch -DCONFIG1 -DCONFIG2 +// RUN: %clang_cc1 -fmodules \ +// RUN: -fmodule-map-file=%S/Inputs/pch-config-macros/include/module.modulemap \ +// RUN: -fmodules-cache-path=%t/cache -I %S/Inputs/pch-config-macros/include \ +// RUN: -include-pch %t/h1.h.pch bridging.h -emit-pch -o bridging.h.pch \ +// RUN: -DCONFIG1 -DCONFIG2 +// RUN: %clang_cc1 -fmodules \ +// RUN: -fmodule-map-file=%S/Inputs/pch-config-macros/include/module.modulemap \ +// RUN: -fmodules-cache-path=%t/cache -I %S/Inputs/pch-config-macros/include \ +// RUN: -emit-obj -o main.o main.c -include-pch %t/bridging.h.pch \ +// RUN: -DCONFIG1 -DCONFIG2 -verify + +//--- h1.h +#if CONFIG1 +int bar1() { return 42; } +#else +int bar2() { return 43; } +#endif + +//--- bridging.h +#if CONFIG1 +int bar() { return bar1(); } +#else +int bar() { return bar2(); } +#endif + +#if CONFIG2 +int baz() { return 77; } +#endif + +//--- main.c +#include "Mod1.h" +// expected-no-diagnostics + +int main_func() { + return foo() + bar(); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
