Skip to content

Conversation

@llvm-beanz
Copy link
Collaborator

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.

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.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2024

@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)

Changes

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 --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]

@github-actions
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 2c95e60df53ba1a5765b3fad9e8ddaff70f21994 5433a9134beb8faf1e2b662c96f76d7e8bc814de --extensions h,c,cpp -- clang/test/Lexer/Inputs/SomeHeaderThatDefinesAwfulThings.h clang/test/Lexer/pragma-scope.c clang/include/clang/Lex/Preprocessor.h clang/lib/Lex/PPMacroExpansion.cpp clang/lib/Lex/Pragma.cpp
View the diff from clang-format here.
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 96240533de..7d9292d072 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1059,7 +1059,8 @@ private:
   /// Warning information for macro annotations.
   llvm::DenseMap<const IdentifierInfo *, MacroAnnotations> AnnotationInfos;
 
-  using MacroScopeVec = llvm::SmallVector<std::pair<IdentifierInfo *, MacroDirective *> >;
+  using MacroScopeVec =
+      llvm::SmallVector<std::pair<IdentifierInfo *, MacroDirective *>>;
   MacroScopeVec *CurScope = nullptr;
   llvm::SmallVector<MacroScopeVec> MacroScopeStack;
 
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index f47a2eb1a3..ecc37bbd01 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -80,9 +80,9 @@ void Preprocessor::popMacroScope(SourceLocation Loc) {
   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);
+      DefMacroDirective *MD =
+          AllocateDefMacroDirective(Prev->getMacroInfo(), Loc);
+      appendMacroDirective(It->first, MD);
     } else {
       UndefMacroDirective *Undef = AllocateUndefMacroDirective(Loc);
       appendMacroDirective(It->first, Undef);
@@ -105,7 +105,7 @@ void Preprocessor::appendMacroDirective(IdentifierInfo *II,
   StoredMD.overrideActiveModuleMacros(*this, II);
 
   if (CurScope)
-    CurScope->push_back(std::make_pair(II,MD));
+    CurScope->push_back(std::make_pair(II, MD));
 
   if (needModuleMacros()) {
     // Track that we created a new macro directive, so we know we should
@@ -1192,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"

@Bigcheese
Copy link
Contributor

Given that some #pragmas also impact Sema things, I think scope is a bit confusing for what it applies to. I think something like macro_scope would be a better name.

As for the extension itself, I think it could be useful, but I'm not sure how our language extension policy applies here for pragmas. This is kind of a general purpose extension and would make the code only usable with Clang, as conditionally applying the pragma would mean the macros still show up and your code breaks (otherwise why would you have the pragma in the first place).

This also hides include guards, and I don't think it does anything about include skipping, so it may behave as if the include guard was still there.

Comment on lines -257 to +288
ArrayRef<ModuleMacro*> Leaf;
ArrayRef<ModuleMacro *> Leaf;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch has a bunch of unrelated formatting changes, so I'm not sure if anything else in this file actually changes.

@vgvassilev
Copy link
Contributor

I wonder if we had an rfc for this. Maybe we should solicit some feedback before moving forward with this extension.

@AaronBallman
Copy link
Collaborator

I wonder if we had an rfc for this. Maybe we should solicit some feedback before moving forward with this extension.

Agreed, I think this should have an RFC for wider discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants