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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
