[clang] Add `pragma clang scope [push|pop]` (PR #121025)

via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 23 20:14:28 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)

<details>
<summary>Changes</summary>

Have you ever had that horrible realization that some header you included defines a macro that matches a commonly used word that appears throughout your codebase? What kind of horrible person would define `max` or `min` as a macro and put it into a public header that ships in an SDK?!?!

Well, I have the solution for you!

Enter "Macro Scopes": with this new preprocessor extension you can wrap pesky includes with `#pragma clang scope push` and `#pragma clang scope pop` to protect your carefully curated source from preprocessor macros that bleed from your dependencies.

---

Patch is 42.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121025.diff


7 Files Affected:

- (modified) clang/docs/LanguageExtensions.rst (+34) 
- (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+2-2) 
- (modified) clang/include/clang/Lex/Preprocessor.h (+7) 
- (modified) clang/lib/Lex/PPMacroExpansion.cpp (+266-233) 
- (modified) clang/lib/Lex/Pragma.cpp (+23-2) 
- (added) clang/test/Lexer/Inputs/SomeHeaderThatDefinesAwfulThings.h (+1) 
- (added) clang/test/Lexer/pragma-scope.c (+36) 


``````````diff
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index cc5f1d4ddf4477..a81fa833eafdc9 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5738,6 +5738,40 @@ in user headers or code. This is controlled by ``-Wpedantic-macros``. Final
 macros will always warn on redefinition, including situations with identical
 bodies and in system headers.
 
+Macro Scope
+===========
+
+Clang supports the pragma ``#pragma clang scope`` which is provided with an
+argument ``push`` or ``pop`` to denote entering and leaving macro scopes. On
+entering a macro scope all macro definitions and undefinitions are recorded so
+that they can be reverted on leaving the scope.
+
+.. code-block:: c
+
+   #define NUM_DOGGOS 2
+
+   #pragma clang scope push
+   #define NUM_DOGGOS 3
+   #pragma clang scope pop // NUM_DOGGOS is restored to 2
+
+   #pragma clang scope push
+   #undef NUM_DOGGOS
+   #pragma clang scope pop // NUM_DOGGOS is restored to 2
+
+   #undef NUM_DOGGOS
+   #pragma clang scope push
+   #define NUM_DOGGOS 1
+   #pragma clang scope pop // NUM_DOGGOS is restored to undefined
+
+A macro scope can be used to wrap header includes to isolate headers from
+leaking macros to the outer source file.
+
+.. code-block:: c
+
+   #pragma clang scope push
+   #include <SomeSystemHeader.h>
+   #pragma clang scope pop // None of the defines from the included header persist.
+
 Line Control
 ============
 
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 959376b0847216..a1f57aafb51bf7 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -693,8 +693,8 @@ def warn_pragma_diagnostic_invalid :
    ExtWarn<"pragma diagnostic expected 'error', 'warning', 'ignored', 'fatal',"
             " 'push', or 'pop'">,
    InGroup<UnknownPragmas>;
-def warn_pragma_diagnostic_cannot_pop :
-   ExtWarn<"pragma diagnostic pop could not pop, no matching push">,
+def warn_pragma_cannot_pop :
+   ExtWarn<"pragma %select{diagnostic|scope}0 pop could not pop, no matching push">,
    InGroup<UnknownPragmas>;
 def warn_pragma_diagnostic_invalid_option :
    ExtWarn<"pragma diagnostic expected option name (e.g. \"-Wundef\")">,
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 3d223c345ea156..96240533deff5e 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1059,6 +1059,10 @@ class Preprocessor {
   /// Warning information for macro annotations.
   llvm::DenseMap<const IdentifierInfo *, MacroAnnotations> AnnotationInfos;
 
+  using MacroScopeVec = llvm::SmallVector<std::pair<IdentifierInfo *, MacroDirective *> >;
+  MacroScopeVec *CurScope = nullptr;
+  llvm::SmallVector<MacroScopeVec> MacroScopeStack;
+
   /// A "freelist" of MacroArg objects that can be
   /// reused for quick allocation.
   MacroArgs *MacroArgCache = nullptr;
@@ -2896,6 +2900,9 @@ class Preprocessor {
     AnnotationInfos[II].FinalAnnotationLoc = AnnotationLoc;
   }
 
+  void pushMacroScope();
+  void popMacroScope(SourceLocation Loc);
+
   const MacroAnnotations &getMacroAnnotations(const IdentifierInfo *II) const {
     return AnnotationInfos.find(II)->second;
   }
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 347c13da0ad215..f47a2eb1a37caf 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -66,7 +66,35 @@ Preprocessor::getLocalMacroDirectiveHistory(const IdentifierInfo *II) const {
                                                 : Pos->second.getLatest();
 }
 
-void Preprocessor::appendMacroDirective(IdentifierInfo *II, MacroDirective *MD){
+void Preprocessor::pushMacroScope() {
+  MacroScopeStack.emplace_back(MacroScopeVec());
+  CurScope = &MacroScopeStack.back();
+}
+
+void Preprocessor::popMacroScope(SourceLocation Loc) {
+  if (!CurScope) {
+    Diag(Loc, diag::warn_pragma_cannot_pop) << /*scope*/ 1;
+    return;
+  }
+
+  for (auto It = CurScope->rbegin(); It != CurScope->rend(); ++It) {
+    MacroDirective *Prev = It->second->getPrevious();
+    if (Prev && Prev->getKind() == MacroDirective::MD_Define) {
+        DefMacroDirective *MD =
+            AllocateDefMacroDirective(Prev->getMacroInfo(), Loc);
+        appendMacroDirective(It->first, MD);
+    } else {
+      UndefMacroDirective *Undef = AllocateUndefMacroDirective(Loc);
+      appendMacroDirective(It->first, Undef);
+    }
+  }
+  // Unwind macro stack...
+  MacroScopeStack.pop_back();
+  CurScope = MacroScopeStack.empty() ? nullptr : &MacroScopeStack.back();
+}
+
+void Preprocessor::appendMacroDirective(IdentifierInfo *II,
+                                        MacroDirective *MD) {
   assert(MD && "MacroDirective should be non-zero!");
   assert(!MD->getPrevious() && "Already attached to a MacroDirective history.");
 
@@ -76,6 +104,9 @@ void Preprocessor::appendMacroDirective(IdentifierInfo *II, MacroDirective *MD){
   StoredMD.setLatest(MD);
   StoredMD.overrideActiveModuleMacros(*this, II);
 
+  if (CurScope)
+    CurScope->push_back(std::make_pair(II,MD));
+
   if (needModuleMacros()) {
     // Track that we created a new macro directive, so we know we should
     // consider building a ModuleMacro for it when we get to the end of
@@ -254,7 +285,7 @@ void Preprocessor::updateModuleMacroInfo(const IdentifierInfo *II,
 }
 
 void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
-  ArrayRef<ModuleMacro*> Leaf;
+  ArrayRef<ModuleMacro *> Leaf;
   auto LeafIt = LeafModuleMacros.find(II);
   if (LeafIt != LeafModuleMacros.end())
     Leaf = LeafIt->second;
@@ -281,11 +312,11 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
   }
 
   // Dump module macros.
-  llvm::DenseSet<ModuleMacro*> Active;
+  llvm::DenseSet<ModuleMacro *> Active;
   for (auto *MM : State ? State->getActiveModuleMacros(*this, II)
                         : ArrayRef<ModuleMacro *>())
     Active.insert(MM);
-  llvm::DenseSet<ModuleMacro*> Visited;
+  llvm::DenseSet<ModuleMacro *> Visited;
   llvm::SmallVector<ModuleMacro *, 16> Worklist(Leaf);
   while (!Worklist.empty()) {
     auto *MM = Worklist.pop_back_val();
@@ -394,7 +425,8 @@ static bool isTrivialSingleTokenExpansion(const MacroInfo *MI,
   IdentifierInfo *II = MI->getReplacementToken(0).getIdentifierInfo();
 
   // If the token isn't an identifier, it's always literally expanded.
-  if (!II) return true;
+  if (!II)
+    return true;
 
   // If the information about this identifier is out of date, update it from
   // the external source.
@@ -411,7 +443,8 @@ static bool isTrivialSingleTokenExpansion(const MacroInfo *MI,
 
   // If this is an object-like macro invocation, it is safe to trivially expand
   // it.
-  if (MI->isObjectLike()) return true;
+  if (MI->isObjectLike())
+    return true;
 
   // If this is a function-like macro invocation, it's safe to trivially expand
   // as long as the identifier is not a macro argument.
@@ -467,7 +500,8 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
   // If this is a macro expansion in the "#if !defined(x)" line for the file,
   // then the macro could expand to different things in other contexts, we need
   // to disable the optimization in this case.
-  if (CurPPLexer) CurPPLexer->MIOpt.ExpandedMacro();
+  if (CurPPLexer)
+    CurPPLexer->MIOpt.ExpandedMacro();
 
   // If this is a builtin macro, like __LINE__ or _Pragma, handle it specially.
   if (MI->isBuiltinMacro()) {
@@ -502,7 +536,8 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     ArgMacro = nullptr;
 
     // If there was an error parsing the arguments, bail out.
-    if (!Args) return true;
+    if (!Args)
+      return true;
 
     ++NumFnMacroExpanded;
   } else {
@@ -540,13 +575,13 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
   // If the macro definition is ambiguous, complain.
   if (M.isAmbiguous()) {
     Diag(Identifier, diag::warn_pp_ambiguous_macro)
-      << Identifier.getIdentifierInfo();
+        << Identifier.getIdentifierInfo();
     Diag(MI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_chosen)
-      << Identifier.getIdentifierInfo();
+        << Identifier.getIdentifierInfo();
     M.forAllDefinitions([&](const MacroInfo *OtherMI) {
       if (OtherMI != MI)
         Diag(OtherMI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_other)
-          << Identifier.getIdentifierInfo();
+            << Identifier.getIdentifierInfo();
     });
   }
 
@@ -556,7 +591,8 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
   // expansion stack, only to take it right back off.
   if (MI->getNumTokens() == 0) {
     // No need for arg info.
-    if (Args) Args->destroy(*this);
+    if (Args)
+      Args->destroy(*this);
 
     // Propagate whitespace info as if we had pushed, then popped,
     // a macro context.
@@ -572,7 +608,8 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     // "#define VAL 42".
 
     // No need for arg info.
-    if (Args) Args->destroy(*this);
+    if (Args)
+      Args->destroy(*this);
 
     // Propagate the isAtStartOfLine/hasLeadingSpace markers of the macro
     // identifier to the expanded token.
@@ -583,14 +620,14 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     Identifier = MI->getReplacementToken(0);
 
     // Restore the StartOfLine/LeadingSpace markers.
-    Identifier.setFlagValue(Token::StartOfLine , isAtStartOfLine);
+    Identifier.setFlagValue(Token::StartOfLine, isAtStartOfLine);
     Identifier.setFlagValue(Token::LeadingSpace, hasLeadingSpace);
 
     // Update the tokens location to include both its expansion and physical
     // locations.
     SourceLocation Loc =
-      SourceMgr.createExpansionLoc(Identifier.getLocation(), ExpandLoc,
-                                   ExpansionEnd,Identifier.getLength());
+        SourceMgr.createExpansionLoc(Identifier.getLocation(), ExpandLoc,
+                                     ExpansionEnd, Identifier.getLength());
     Identifier.setLocation(Loc);
 
     // If this is a disabled macro or #define X X, we must mark the result as
@@ -617,10 +654,7 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
   return false;
 }
 
-enum Bracket {
-  Brace,
-  Paren
-};
+enum Bracket { Brace, Paren };
 
 /// CheckMatchedBrackets - Returns true if the braces and parentheses in the
 /// token vector are properly nested.
@@ -728,8 +762,8 @@ static bool GenerateNewArgTokens(Preprocessor &PP,
           TempToken.setLocation(Loc);
           TempToken.setLength(0);
           NewTokens.push_back(TempToken);
-          ParenHints.push_back(SourceRange(ArgStartIterator->getLocation(),
-                                           Loc));
+          ParenHints.push_back(
+              SourceRange(ArgStartIterator->getLocation(), Loc));
         }
 
         // Copy separator token
@@ -797,7 +831,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
         if (!ContainsCodeCompletionTok) {
           Diag(MacroName, diag::err_unterm_macro_invoc);
           Diag(MI->getDefinitionLoc(), diag::note_macro_here)
-            << MacroName.getIdentifierInfo();
+              << MacroName.getIdentifierInfo();
           // Do not lose the EOF/EOD.  Return it to the client.
           MacroName = Tok;
           return nullptr;
@@ -811,8 +845,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
         // If we found the ) token, the macro arg list is done.
         if (NumParens-- == 0) {
           MacroEnd = Tok.getLocation();
-          if (!ArgTokens.empty() &&
-              ArgTokens.back().commaAfterElided()) {
+          if (!ArgTokens.empty() && ArgTokens.back().commaAfterElided()) {
             FoundElidedComma = true;
           }
           break;
@@ -911,7 +944,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
     // Emitting it at the , could be far away from the macro name.
     Diag(TooManyArgsLoc, diag::err_too_many_args_in_macro_invoc);
     Diag(MI->getDefinitionLoc(), diag::note_macro_here)
-      << MacroName.getIdentifierInfo();
+        << MacroName.getIdentifierInfo();
 
     // Commas from braced initializer lists will be treated as argument
     // separators inside macros.  Attempt to correct for this with parentheses.
@@ -924,9 +957,8 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
     if (!GenerateNewArgTokens(*this, ArgTokens, FixedArgTokens, FixedNumArgs,
                               ParenHints, InitLists)) {
       if (!InitLists.empty()) {
-        DiagnosticBuilder DB =
-            Diag(MacroName,
-                 diag::note_init_list_at_beginning_of_macro_argument);
+        DiagnosticBuilder DB = Diag(
+            MacroName, diag::note_init_list_at_beginning_of_macro_argument);
         for (SourceRange Range : InitLists)
           DB << Range;
       }
@@ -968,8 +1000,8 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
       // the macro expects one argument (the argument is just empty).
       isVarargsElided = MI->isVariadic();
     } else if ((FoundElidedComma || MI->isVariadic()) &&
-               (NumActuals+1 == MinArgsExpected ||  // A(x, ...) -> A(X)
-                (NumActuals == 0 && MinArgsExpected == 2))) {// A(x,...) -> A()
+               (NumActuals + 1 == MinArgsExpected || // A(x, ...) -> A(X)
+                (NumActuals == 0 && MinArgsExpected == 2))) { // A(x,...) -> A()
       // Varargs where the named vararg parameter is missing: OK as extension.
       //   #define A(x, ...)
       //   A("blah")
@@ -992,7 +1024,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
           ID = diag::ext_c_missing_varargs_arg;
         Diag(Tok, ID);
         Diag(MI->getDefinitionLoc(), diag::note_macro_here)
-          << MacroName.getIdentifierInfo();
+            << MacroName.getIdentifierInfo();
       }
 
       // Remember this occurred, allowing us to elide the comma when used for
@@ -1006,7 +1038,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
       // Otherwise, emit the error.
       Diag(Tok, diag::err_too_few_args_in_macro_invoc);
       Diag(MI->getDefinitionLoc(), diag::note_macro_here)
-        << MacroName.getIdentifierInfo();
+          << MacroName.getIdentifierInfo();
       return nullptr;
     }
 
@@ -1028,7 +1060,7 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
     // Emitting it at the , could be far away from the macro name.
     Diag(MacroName, diag::err_too_many_args_in_macro_invoc);
     Diag(MI->getDefinitionLoc(), diag::note_macro_here)
-      << MacroName.getIdentifierInfo();
+        << MacroName.getIdentifierInfo();
     return nullptr;
   }
 
@@ -1047,8 +1079,8 @@ Token *Preprocessor::cacheMacroExpandedTokens(TokenLexer *tokLexer,
     return nullptr;
 
   size_t newIndex = MacroExpandedTokens.size();
-  bool cacheNeedsToGrow = tokens.size() >
-                      MacroExpandedTokens.capacity()-MacroExpandedTokens.size();
+  bool cacheNeedsToGrow = tokens.size() > MacroExpandedTokens.capacity() -
+                                              MacroExpandedTokens.size();
   MacroExpandedTokens.append(tokens.begin(), tokens.end());
 
   if (cacheNeedsToGrow) {
@@ -1090,9 +1122,9 @@ static void ComputeDATE_TIME(SourceLocation &DATELoc, SourceLocation &TIMELoc,
     TM = std::localtime(&TT);
   }
 
-  static const char * const Months[] = {
-    "Jan","Feb","Mar","Apr","May","Jun","Jul","Aug","Sep","Oct","Nov","Dec"
-  };
+  static const char *const Months[] = {"Jan", "Feb", "Mar", "Apr",
+                                       "May", "Jun", "Jul", "Aug",
+                                       "Sep", "Oct", "Nov", "Dec"};
 
   {
     SmallString<32> TmpBuffer;
@@ -1160,8 +1192,8 @@ static bool HasExtension(const Preprocessor &PP, StringRef Extension) {
       Extension.size() >= 4)
     Extension = Extension.substr(2, Extension.size() - 4);
 
-    // Because we inherit the feature list from HasFeature, this string switch
-    // must be less restrictive than HasFeature's.
+  // Because we inherit the feature list from HasFeature, this string switch
+  // must be less restrictive than HasFeature's.
 #define EXTENSION(Name, Predicate) .Case(#Name, Predicate)
   return llvm::StringSwitch<bool>(Extension)
 #include "clang/Basic/Features.def"
@@ -1376,17 +1408,15 @@ bool Preprocessor::EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II) {
 
 /// Process single-argument builtin feature-like macros that return
 /// integer values.
-static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
-                                            Token &Tok, IdentifierInfo *II,
-                                            Preprocessor &PP, bool ExpandArgs,
-                                            llvm::function_ref<
-                                              int(Token &Tok,
-                                                  bool &HasLexedNextTok)> Op) {
+static void EvaluateFeatureLikeBuiltinMacro(
+    llvm::raw_svector_ostream &OS, Token &Tok, IdentifierInfo *II,
+    Preprocessor &PP, bool ExpandArgs,
+    llvm::function_ref<int(Token &Tok, bool &HasLexedNextTok)> Op) {
   // Parse the initial '('.
   PP.LexUnexpandedToken(Tok);
   if (Tok.isNot(tok::l_paren)) {
-    PP.Diag(Tok.getLocation(), diag::err_pp_expected_after) << II
-                                                            << tok::l_paren;
+    PP.Diag(Tok.getLocation(), diag::err_pp_expected_after)
+        << II << tok::l_paren;
 
     // Provide a dummy '0' value on output stream to elide further errors.
     if (!Tok.isOneOf(tok::eof, tok::eod)) {
@@ -1409,64 +1439,64 @@ static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
     else
       PP.LexUnexpandedToken(Tok);
 
-already_lexed:
+  already_lexed:
     switch (Tok.getKind()) {
-      case tok::eof:
-      case tok::eod:
-        // Don't provide even a dummy value if the eod or eof marker is
-        // reached.  Simply provide a diagnostic.
-        PP.Diag(Tok.getLocation(), diag::err_unterm_macro_invoc);
-        return;
+    case tok::eof:
+    case tok::eod:
+      // Don't provide even a dummy value if the eod or eof marker is
+      // reached.  Simply provide a diagnostic.
+      PP.Diag(Tok.getLocation(), diag::err_unterm_macro_invoc);
+      return;
 
-      case tok::comma:
-        if (!SuppressDiagnostic) {
-          PP.Diag(Tok.getLocation(), diag::err_too_many_args_in_macro_invoc);
-          SuppressDiagnostic = true;
-        }
-        continue;
+    case tok::comma:
+      if (!SuppressDiagnostic) {
+        PP.Diag(Tok.getLocation(), diag::err_too_many_args_in_macro_invoc);
+        SuppressDiagnostic = true;
+      }
+      continue;
 
-      case tok::l_paren:
-        ++ParenDepth;
-        if (Result)
-          break;
-        if (!SuppressDiagnostic) {
-          PP.Diag(Tok.getLocation(), diag::err_pp_nested_paren) << II;
-          SuppressDiagnostic = true;
-        }
+    case tok::l_paren:
+      ++ParenDepth;
+      if (Result)
+        break;
+      if (!SuppressDiagnostic) {
+        PP.Diag(Tok.getLocation(), diag::err_pp_nested_paren) << II;
+        SuppressDiagnostic = true;
+      }
+      continue;
+
+    case tok::r_paren:
+      if (--ParenDepth > 0)
         continue;
 
-      case tok::r_paren:
-        if (--ParenDepth > 0)
-          continue;
-
-        // The last ')' has been reached; return the value if one found or
-        // a diagnostic and a dummy value.
-        if (Result) {
-          OS << *Result;
-          // For strict conformance to __has_cpp_attribute rules, use 'L'
-          // suffix for dated literals.
-          if (*Result > 1)
-            OS << 'L';
-        } else {
-          OS << 0;
-          if (!SuppressDiagnostic)
-            PP.Diag(Tok.getLocation(), diag::err_too_few_args_in_macro_invoc);
-        }
-        Tok.setKind(tok::numeric_constant);
-        return;
+      // The last ')' has been reached; return the value if one found or
+      // a diagnostic and a dummy value.
+      if (Result) {
+        OS << *Result;
+        // For strict conformance to __has_cpp_attribute rules, use 'L'
+        // suffix for dated li...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/121025


More information about the cfe-commits mailing list