[clang] [Preprocessor] Do not expand macros if the input is already preprocessed (PR #137665)

Juan Manuel Martinez CaamaƱo via cfe-commits cfe-commits at lists.llvm.org
Tue May 27 06:08:39 PDT 2025


https://github.com/jmmartinez updated https://github.com/llvm/llvm-project/pull/137665

>From f416b07520e73c7feec72378d970305e85295ad8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Mon, 28 Apr 2025 17:05:46 +0200
Subject: [PATCH 1/6] Pre-commit test: [Preprocessor] Do not expand macros if
 the input is already preprocessed

---
 clang/test/Preprocessor/preprocess-cpp-output.c | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 clang/test/Preprocessor/preprocess-cpp-output.c

diff --git a/clang/test/Preprocessor/preprocess-cpp-output.c b/clang/test/Preprocessor/preprocess-cpp-output.c
new file mode 100644
index 0000000000000..59ff057e9b871
--- /dev/null
+++ b/clang/test/Preprocessor/preprocess-cpp-output.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -E -x c %s | FileCheck %s --check-prefixes=EXPANDED
+// RUN: %clang_cc1 -E -x cpp-output %s | FileCheck %s --check-prefixes=EXPANDED
+
+// EXPANDED: void __attribute__((__attribute__((always_inline)))) foo()
+
+#define always_inline __attribute__((always_inline))
+void __attribute__((always_inline)) foo() {
+    return 4;
+}

>From 21fc4802b6147dd4c21cc7163f1a2fe8595f28c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Tue, 13 May 2025 15:03:46 +0200
Subject: [PATCH 2/6] [Modules] initializers.cpp test fix

The module contents should not contain preprocessor directives. The
contents should be already preprocessed.

Duplicate the modules instead to propose 2 versions: one with the
namespace ns and one without.
---
 clang/test/Modules/initializers.cpp | 59 +++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/clang/test/Modules/initializers.cpp b/clang/test/Modules/initializers.cpp
index dcd9b08ec6f7a..e1f826fbc09f3 100644
--- a/clang/test/Modules/initializers.cpp
+++ b/clang/test/Modules/initializers.cpp
@@ -48,6 +48,7 @@
 // instantiation for v<int> in one of the two headers, because we will only
 // parse one of the two get() functions.
 
+#ifdef NS
 #pragma clang module build m
 module m {
   module a {
@@ -60,9 +61,7 @@ module m {
 #pragma clang module begin m.a
 inline int non_trivial() { return 3; }
 
-#ifdef NS
 namespace ns {
-#endif
 
 int a = non_trivial();
 inline int b = non_trivial();
@@ -102,12 +101,64 @@ inline void use(bool b, ...) {
       X<int>::e<int>, X<int>::f<int>, X<int>::g<int>, X<int>::h<int>);
 }
 
-#ifdef NS
 }
-#endif
 
 #pragma clang module end
 #pragma clang module endbuild
+#else
+#pragma clang module build m
+module m {
+  module a {
+    header "foo.h" { size 123 mtime 456789 }
+  }
+  module b {}
+}
+
+#pragma clang module contents
+#pragma clang module begin m.a
+inline int non_trivial() { return 3; }
+
+int a = non_trivial();
+inline int b = non_trivial();
+thread_local int c = non_trivial();
+inline thread_local int d = non_trivial();
+
+template<typename U> int e = non_trivial();
+template<typename U> inline int f = non_trivial();
+template<typename U> thread_local int g = non_trivial();
+template<typename U> inline thread_local int h = non_trivial();
+
+inline int unused = 123; // should not be emitted
+
+template<typename T> struct X {
+  static int a;
+  static inline int b = non_trivial();
+  static thread_local int c;
+  static inline thread_local int d = non_trivial();
+
+  template<typename U> static int e;
+  template<typename U> static inline int f = non_trivial();
+  template<typename U> static thread_local int g;
+  template<typename U> static inline thread_local int h = non_trivial();
+
+  static inline int unused = 123; // should not be emitted
+};
+
+template<typename T> int X<T>::a = non_trivial();
+template<typename T> thread_local int X<T>::c = non_trivial();
+template<typename T> template<typename U> int X<T>::e = non_trivial();
+template<typename T> template<typename U> thread_local int X<T>::g = non_trivial();
+
+inline void use(bool b, ...) {
+  if (b) return;
+  use(true, e<int>, f<int>, g<int>, h<int>,
+      X<int>::a, X<int>::b, X<int>::c, X<int>::d,
+      X<int>::e<int>, X<int>::f<int>, X<int>::g<int>, X<int>::h<int>);
+}
+
+#pragma clang module end
+#pragma clang module endbuild
+#endif
 
 #if IMPORT == 1
 // Import the module and the m.a submodule; runs the ordered initializers and

>From 2492698a0ecdefbc25bf3be399f053e697eb6df9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Tue, 22 Apr 2025 18:40:37 +0200
Subject: [PATCH 3/6] [Preprocessor] Do not expand macros if the input is
 already preprocessed

---
 clang/include/clang/Lex/Preprocessor.h          | 5 +++++
 clang/lib/Frontend/InitPreprocessor.cpp         | 7 +++++++
 clang/test/Preprocessor/preprocess-cpp-output.c | 3 ++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index f2dfd3a349b8b..63774e48a468b 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1831,6 +1831,11 @@ class Preprocessor {
     MacroExpansionInDirectivesOverride = true;
   }
 
+  void SetDisableMacroExpansion() {
+    DisableMacroExpansion = true;
+    MacroExpansionInDirectivesOverride = false;
+  }
+
   /// Peeks ahead N tokens and returns that token without consuming any
   /// tokens.
   ///
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 96d6fb64a6319..2840632030919 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1558,6 +1558,13 @@ void clang::InitializePreprocessor(Preprocessor &PP,
                                    const PCHContainerReader &PCHContainerRdr,
                                    const FrontendOptions &FEOpts,
                                    const CodeGenOptions &CodeGenOpts) {
+
+  if (all_of(FEOpts.Inputs,
+             [](const FrontendInputFile &FI) { return FI.isPreprocessed(); })) {
+    PP.SetDisableMacroExpansion();
+    return;
+  }
+
   const LangOptions &LangOpts = PP.getLangOpts();
   std::string PredefineBuffer;
   PredefineBuffer.reserve(4080);
diff --git a/clang/test/Preprocessor/preprocess-cpp-output.c b/clang/test/Preprocessor/preprocess-cpp-output.c
index 59ff057e9b871..2c180601e30ac 100644
--- a/clang/test/Preprocessor/preprocess-cpp-output.c
+++ b/clang/test/Preprocessor/preprocess-cpp-output.c
@@ -1,7 +1,8 @@
 // RUN: %clang_cc1 -E -x c %s | FileCheck %s --check-prefixes=EXPANDED
-// RUN: %clang_cc1 -E -x cpp-output %s | FileCheck %s --check-prefixes=EXPANDED
+// RUN: %clang_cc1 -E -x cpp-output %s | FileCheck %s --check-prefixes=NOT-EXPANDED
 
 // EXPANDED: void __attribute__((__attribute__((always_inline)))) foo()
+// NOT-EXPANDED: void __attribute__((always_inline)) foo()
 
 #define always_inline __attribute__((always_inline))
 void __attribute__((always_inline)) foo() {

>From 3d1ce5b0d34275eeacad58770b39b193bc249c9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Thu, 22 May 2025 16:55:40 +0200
Subject: [PATCH 4/6] [Review] Disable macro expansion in BeginSourceFileAction

---
 clang/include/clang/Frontend/FrontendAction.h | 2 ++
 clang/lib/Frontend/InitPreprocessor.cpp       | 7 -------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h
index 718684a67771a..40a9389bbf81a 100644
--- a/clang/include/clang/Frontend/FrontendAction.h
+++ b/clang/include/clang/Frontend/FrontendAction.h
@@ -84,6 +84,8 @@ class FrontendAction {
   /// \return True on success; on failure ExecutionAction() and
   /// EndSourceFileAction() will not be called.
   virtual bool BeginSourceFileAction(CompilerInstance &CI) {
+    if (CurrentInput.isPreprocessed())
+      CI.getPreprocessor().SetDisableMacroExpansion();
     return true;
   }
 
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 2840632030919..96d6fb64a6319 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1558,13 +1558,6 @@ void clang::InitializePreprocessor(Preprocessor &PP,
                                    const PCHContainerReader &PCHContainerRdr,
                                    const FrontendOptions &FEOpts,
                                    const CodeGenOptions &CodeGenOpts) {
-
-  if (all_of(FEOpts.Inputs,
-             [](const FrontendInputFile &FI) { return FI.isPreprocessed(); })) {
-    PP.SetDisableMacroExpansion();
-    return;
-  }
-
   const LangOptions &LangOpts = PP.getLangOpts();
   std::string PredefineBuffer;
   PredefineBuffer.reserve(4080);

>From 6da133ff7939451d5a0ec29f157ad8695f06c1e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Mon, 26 May 2025 15:26:30 +0200
Subject: [PATCH 5/6] BeginSourceFileAction/EndSourceFileAction invoke the
 base-class's method

An exception for this is the WrapperFrontendAction which cover this
through the fowraded call to WrappedAction->BeginSourceFileAction/EndSourcefileAction
---
 clang/lib/CodeGen/CodeGenAction.cpp            | 4 +++-
 clang/lib/Frontend/FrontendActions.cpp         | 2 +-
 clang/lib/Frontend/Rewrite/FrontendActions.cpp | 5 +++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 1f5eb427b566f..be6e34649431d 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -908,6 +908,8 @@ bool CodeGenAction::loadLinkModules(CompilerInstance &CI) {
 bool CodeGenAction::hasIRSupport() const { return true; }
 
 void CodeGenAction::EndSourceFileAction() {
+  ASTFrontendAction::EndSourceFileAction();
+
   // If the consumer creation failed, do nothing.
   if (!getCompilerInstance().hasASTConsumer())
     return;
@@ -932,7 +934,7 @@ CodeGenerator *CodeGenAction::getCodeGenerator() const {
 bool CodeGenAction::BeginSourceFileAction(CompilerInstance &CI) {
   if (CI.getFrontendOpts().GenReducedBMI)
     CI.getLangOpts().setCompilingModule(LangOptions::CMK_ModuleInterface);
-  return true;
+  return ASTFrontendAction::BeginSourceFileAction(CI);
 }
 
 static std::unique_ptr<raw_pwrite_stream>
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index d14d091e3fe38..fe3ff67eb3b8b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -182,7 +182,7 @@ bool GeneratePCHAction::shouldEraseOutputFiles() {
 
 bool GeneratePCHAction::BeginSourceFileAction(CompilerInstance &CI) {
   CI.getLangOpts().CompilingPCH = true;
-  return true;
+  return ASTFrontendAction::BeginSourceFileAction(CI);
 }
 
 std::vector<std::unique_ptr<ASTConsumer>>
diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
index f955845c9cbf2..5583a86204b97 100644
--- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -104,12 +104,13 @@ bool FixItAction::BeginSourceFileAction(CompilerInstance &CI) {
   }
   Rewriter.reset(new FixItRewriter(CI.getDiagnostics(), CI.getSourceManager(),
                                    CI.getLangOpts(), FixItOpts.get()));
-  return true;
+  return ASTFrontendAction::BeginSourceFileAction(CI);
 }
 
 void FixItAction::EndSourceFileAction() {
   // Otherwise rewrite all files.
   Rewriter->WriteFixedFiles();
+  ASTFrontendAction::EndSourceFileAction();
 }
 
 bool FixItRecompile::BeginInvocation(CompilerInstance &CI) {
@@ -299,7 +300,7 @@ bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance &CI) {
         std::make_unique<RewriteImportsListener>(CI, OutputStream));
   }
 
-  return true;
+  return PreprocessorFrontendAction::BeginSourceFileAction(CI);
 }
 
 void RewriteIncludesAction::ExecuteAction() {

>From 89ace0798daa75e08cff4bdbb21f770767925b13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?= <juamarti at amd.com>
Date: Mon, 26 May 2025 16:48:12 +0200
Subject: [PATCH 6/6] [WIP] Disable and Restore the Preprocessor's macro
 expansion in FrontendAction's BeginSourceFileAction and EndSourceFileAction

---
 clang/include/clang/Frontend/FrontendAction.h | 8 ++++++--
 clang/include/clang/Lex/Preprocessor.h        | 5 +----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h
index 40a9389bbf81a..66585dbacbb36 100644
--- a/clang/include/clang/Frontend/FrontendAction.h
+++ b/clang/include/clang/Frontend/FrontendAction.h
@@ -85,7 +85,7 @@ class FrontendAction {
   /// EndSourceFileAction() will not be called.
   virtual bool BeginSourceFileAction(CompilerInstance &CI) {
     if (CurrentInput.isPreprocessed())
-      CI.getPreprocessor().SetDisableMacroExpansion();
+      CI.getPreprocessor().SetEnableMacroExpansion(false);
     return true;
   }
 
@@ -100,7 +100,11 @@ class FrontendAction {
   ///
   /// This is guaranteed to only be called following a successful call to
   /// BeginSourceFileAction (and BeginSourceFile).
-  virtual void EndSourceFileAction() {}
+  virtual void EndSourceFileAction() {
+    if (CurrentInput.isPreprocessed())
+      // reset the preprocessor macro expansion to the default
+      getCompilerInstance().getPreprocessor().SetEnableMacroExpansion(true);
+  }
 
   /// Callback at the end of processing a single input, to determine
   /// if the output files should be erased or not.
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 63774e48a468b..856946ba38c2b 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1831,10 +1831,7 @@ class Preprocessor {
     MacroExpansionInDirectivesOverride = true;
   }
 
-  void SetDisableMacroExpansion() {
-    DisableMacroExpansion = true;
-    MacroExpansionInDirectivesOverride = false;
-  }
+  void SetEnableMacroExpansion(bool Enable) { DisableMacroExpansion = !Enable; }
 
   /// Peeks ahead N tokens and returns that token without consuming any
   /// tokens.



More information about the cfe-commits mailing list