https://github.com/davidstone updated https://github.com/llvm/llvm-project/pull/197817
>From 0861446c4e9a54238ce0749b050c8e4a3c4b5782 Mon Sep 17 00:00:00 2001 From: David Stone <[email protected]> Date: Thu, 14 May 2026 15:34:18 -0600 Subject: [PATCH 1/3] [clang][NFC] Remove `macro_begin` and `macro_end` from `Preprocessor` Use `macros()` wherever posible. This gives us the following advantages: 1. We can use the range-base for loop for simpler looking code. 2. We more ergonomically use algorithms. 3. We can avoid the duplicate work of checking if we need to call `ExternalSource->ReadDefinedMacros()` that was in both `macro_begin` and `macro_end`. In some cases, we save this extra work once per loop iteration, not just one extra total. 4. No user confusion deciding which version they should call 5. Reduce the size of `Preprocessor.h` and the number of members in `Preprocessor`. Using `macros` ends up being the better solution in every case, so since all callers were migrated from `macro_begin` and `macro_end` to `macros`, get rid of `macro_begin` and `macro_end`. --- .../bugprone/BadSignalToKillThreadCheck.cpp | 7 +-- .../bugprone/NotNullTerminatedResultCheck.cpp | 43 ++++++++----------- clang-tools-extra/modularize/Modularize.cpp | 8 ++-- clang/include/clang/Lex/Preprocessor.h | 9 +--- .../lib/Frontend/PrintPreprocessedOutput.cpp | 7 ++- clang/lib/Lex/Preprocessor.cpp | 27 +++--------- clang/lib/Sema/SemaCodeComplete.cpp | 16 +++---- .../Clang/ClangModulesDeclVendor.cpp | 23 ++++------ 8 files changed, 50 insertions(+), 90 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp index 43a352e389f68..e9cf61e5abf89 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp @@ -32,9 +32,10 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { return KeyValue.first->getName() == "SIGTERM" && KeyValue.first->hasMacroDefinition(); }; + const auto macros = PP->macros(); const auto TryExpandAsInteger = - [](Preprocessor::macro_iterator It) -> std::optional<unsigned> { - if (It == PP->macro_end()) + [&](Preprocessor::macro_iterator It) -> std::optional<unsigned> { + if (It == macros.end()) return std::nullopt; const MacroInfo *MI = PP->getMacroInfo(It->first); const Token &T = MI->tokens().back(); @@ -55,7 +56,7 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { return IntValue.getZExtValue(); }; - const auto SigtermMacro = llvm::find_if(PP->macros(), IsSigterm); + const auto SigtermMacro = llvm::find_if(macros, IsSigterm); if (!SigtermValue && !(SigtermValue = TryExpandAsInteger(SigtermMacro))) return; diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp index eec567dfb2a61..e12a7cf883d46 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp @@ -297,15 +297,10 @@ static void lengthExprHandle(const Expr *LengthExpr, LengthExpr = LengthExpr->IgnoreParenImpCasts(); // See whether we work with a macro. - bool IsMacroDefinition = false; const StringRef LengthExprStr = exprToStr(LengthExpr, Result); - Preprocessor::macro_iterator It = PP->macro_begin(); - while (It != PP->macro_end() && !IsMacroDefinition) { - if (It->first->getName() == LengthExprStr) - IsMacroDefinition = true; - - ++It; - } + bool IsMacroDefinition = llvm::any_of(PP->macros(), [=](const auto &M) { + return M.first->getName() == LengthExprStr; + }); // Try to obtain an 'IntegerLiteral' and adjust it. if (!IsMacroDefinition) { @@ -796,25 +791,21 @@ void NotNullTerminatedResultCheck::check( if (WantToUseSafeFunctions && PP->isMacroDefined("__STDC_LIB_EXT1__")) { std::optional<bool> AreSafeFunctionsWanted; - - Preprocessor::macro_iterator It = PP->macro_begin(); - while (It != PP->macro_end() && !AreSafeFunctionsWanted) { - if (It->first->getName() == "__STDC_WANT_LIB_EXT1__") { - const auto *MI = PP->getMacroInfo(It->first); - // PP->getMacroInfo() returns nullptr if macro has no definition. - if (MI) { - const auto &T = MI->tokens().back(); - if (T.isLiteral() && T.getLiteralData()) { - const StringRef ValueStr = - StringRef(T.getLiteralData(), T.getLength()); - llvm::APInt IntValue; - ValueStr.getAsInteger(10, IntValue); - AreSafeFunctionsWanted = IntValue.getZExtValue(); - } - } + for (const auto &M : PP->macros()) { + if (M.first->getName() != "__STDC_WANT_LIB_EXT1__") + continue; + const auto *MI = PP->getMacroInfo(M.first); + // PP->getMacroInfo() returns nullptr if macro has no definition. + if (!MI) + continue; + const auto &T = MI->tokens().back(); + if (T.isLiteral() && T.getLiteralData()) { + const StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); + llvm::APInt IntValue; + ValueStr.getAsInteger(10, IntValue); + AreSafeFunctionsWanted = IntValue.getZExtValue(); + break; } - - ++It; } if (AreSafeFunctionsWanted) diff --git a/clang-tools-extra/modularize/Modularize.cpp b/clang-tools-extra/modularize/Modularize.cpp index 33966b44f719a..5af50c71d1c16 100644 --- a/clang-tools-extra/modularize/Modularize.cpp +++ b/clang-tools-extra/modularize/Modularize.cpp @@ -653,14 +653,12 @@ class CollectEntitiesConsumer : public ASTConsumer { .TraverseDecl(Ctx.getTranslationUnitDecl()); // Collect macro definitions. - for (Preprocessor::macro_iterator M = PP.macro_begin(), - MEnd = PP.macro_end(); - M != MEnd; ++M) { - Location Loc(SM, M->second.getLatest()->getLocation()); + for (const auto &M : PP.macros()) { + Location Loc(SM, M.second.getLatest()->getLocation()); if (!Loc) continue; - Entities.add(M->first->getName().str(), Entry::EK_Macro, Loc); + Entities.add(M.first->getName().str(), Entry::EK_Macro, Loc); } // Merge header contents. diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 8cba21539e48a..c0c94b7ea4101 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -1518,15 +1518,8 @@ class Preprocessor { /// MacroInfo::getUndefLoc() at the head of the list. using macro_iterator = MacroMap::const_iterator; - macro_iterator macro_begin(bool IncludeExternalMacros = true) const; - macro_iterator macro_end(bool IncludeExternalMacros = true) const; - llvm::iterator_range<macro_iterator> - macros(bool IncludeExternalMacros = true) const { - macro_iterator begin = macro_begin(IncludeExternalMacros); - macro_iterator end = macro_end(IncludeExternalMacros); - return llvm::make_range(begin, end); - } + macros(bool IncludeExternalMacros = true) const; /// \} diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp index 0dc8a86e604d3..02266882c4c4a 100644 --- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp +++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp @@ -1081,11 +1081,10 @@ static void DoPrintMacros(Preprocessor &PP, raw_ostream *OS) { PP.LexTokensUntilEOF(); SmallVector<id_macro_pair, 128> MacrosByID; - for (Preprocessor::macro_iterator I = PP.macro_begin(), E = PP.macro_end(); - I != E; ++I) { - auto *MD = I->second.getLatest(); + for (const auto &M : PP.macros()) { + auto *MD = M.second.getLatest(); if (MD && MD->isDefined()) - MacrosByID.push_back(id_macro_pair(I->first, MD->getMacroInfo())); + MacrosByID.push_back(id_macro_pair(M.first, MD->getMacroInfo())); } llvm::array_pod_sort(MacrosByID.begin(), MacrosByID.end(), MacroIDCompare); diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp index 761bf8e9af56b..d190f79a71788 100644 --- a/clang/lib/Lex/Preprocessor.cpp +++ b/clang/lib/Lex/Preprocessor.cpp @@ -357,19 +357,18 @@ void Preprocessor::PrintStats() { << llvm::capacity_in_bytes(CommentHandlers) << "\n"; } -Preprocessor::macro_iterator -Preprocessor::macro_begin(bool IncludeExternalMacros) const { +llvm::iterator_range<Preprocessor::macro_iterator> +Preprocessor::macros(bool IncludeExternalMacros) const { if (IncludeExternalMacros && ExternalSource && !ReadMacrosFromExternalSource) { ReadMacrosFromExternalSource = true; ExternalSource->ReadDefinedMacros(); } - // Make sure we cover all macros in visible modules. for (const ModuleMacro &Macro : ModuleMacros) CurSubmoduleState->Macros.try_emplace(Macro.II); - return CurSubmoduleState->Macros.begin(); + return CurSubmoduleState->Macros; } size_t Preprocessor::getTotalMemory() const { @@ -384,17 +383,6 @@ size_t Preprocessor::getTotalMemory() const { + llvm::capacity_in_bytes(CommentHandlers); } -Preprocessor::macro_iterator -Preprocessor::macro_end(bool IncludeExternalMacros) const { - if (IncludeExternalMacros && ExternalSource && - !ReadMacrosFromExternalSource) { - ReadMacrosFromExternalSource = true; - ExternalSource->ReadDefinedMacros(); - } - - return CurSubmoduleState->Macros.end(); -} - /// Compares macro tokens with a specified token value sequence. static bool MacroDefinitionEquals(const MacroInfo *MI, ArrayRef<TokenValue> Tokens) { @@ -407,10 +395,9 @@ StringRef Preprocessor::getLastMacroWithSpelling( ArrayRef<TokenValue> Tokens) const { SourceLocation BestLocation; StringRef BestSpelling; - for (Preprocessor::macro_iterator I = macro_begin(), E = macro_end(); - I != E; ++I) { - const MacroDirective::DefInfo - Def = I->second.findDirectiveAtLoc(Loc, SourceMgr); + for (const auto &M : macros()) { + const MacroDirective::DefInfo Def = + M.second.findDirectiveAtLoc(Loc, SourceMgr); if (!Def || !Def.getMacroInfo()) continue; if (!Def.getMacroInfo()->isObjectLike()) @@ -423,7 +410,7 @@ StringRef Preprocessor::getLastMacroWithSpelling( (Location.isValid() && SourceMgr.isBeforeInTranslationUnit(BestLocation, Location))) { BestLocation = Location; - BestSpelling = I->first->getName(); + BestSpelling = M.first->getName(); } } return BestSpelling; diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 74264ec3906ff..cf07a9cb68d60 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -4513,18 +4513,16 @@ static void AddMacroResults(Preprocessor &PP, ResultBuilder &Results, Results.EnterNewScope(); - for (Preprocessor::macro_iterator M = PP.macro_begin(LoadExternal), - MEnd = PP.macro_end(LoadExternal); - M != MEnd; ++M) { - auto MD = PP.getMacroDefinition(M->first); + for (const auto &M : PP.macros(LoadExternal)) { + auto MD = PP.getMacroDefinition(M.first); if (IncludeUndefined || MD) { MacroInfo *MI = MD.getMacroInfo(); if (MI && MI->isUsedForHeaderGuard()) continue; Results.AddResult( - Result(M->first, MI, - getMacroUsagePriority(M->first->getName(), PP.getLangOpts(), + Result(M.first, MI, + getMacroUsagePriority(M.first->getName(), PP.getLangOpts(), TargetTypeIsPointer))); } } @@ -10365,11 +10363,9 @@ void SemaCodeCompletion::CodeCompletePreprocessorMacroName(bool IsDefinition) { CodeCompletionBuilder Builder(Results.getAllocator(), Results.getCodeCompletionTUInfo()); Results.EnterNewScope(); - for (Preprocessor::macro_iterator M = SemaRef.PP.macro_begin(), - MEnd = SemaRef.PP.macro_end(); - M != MEnd; ++M) { + for (const auto &M : SemaRef.PP.macros()) { Builder.AddTypedTextChunk( - Builder.getAllocator().CopyString(M->first->getName())); + Builder.getAllocator().CopyString(M.first->getName())); Results.AddResult(CodeCompletionResult( Builder.TakeString(), CCP_CodePattern, CXCursor_MacroDefinition)); } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp index 2f2a9e689acb6..3784acd511095 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp @@ -513,22 +513,17 @@ void ClangModulesDeclVendorImpl::ForEachMacro( ->ReadDefinedMacros(); } - for (clang::Preprocessor::macro_iterator - mi = m_compiler_instance->getPreprocessor().macro_begin(), - me = m_compiler_instance->getPreprocessor().macro_end(); - mi != me; ++mi) { + for (const auto &m : m_compiler_instance->getPreprocessor().macros()) { const clang::IdentifierInfo *ii = nullptr; - { - if (clang::IdentifierInfoLookup *lookup = - m_compiler_instance->getPreprocessor() - .getIdentifierTable() - .getExternalIdentifierLookup()) { - lookup->get(mi->first->getName()); - } - if (!ii) - ii = mi->first; + if (clang::IdentifierInfoLookup *lookup = + m_compiler_instance->getPreprocessor() + .getIdentifierTable() + .getExternalIdentifierLookup()) { + lookup->get(m.first->getName()); } + if (!ii) + ii = m.first; ssize_t found_priority = -1; clang::MacroInfo *macro_info = nullptr; @@ -562,7 +557,7 @@ void ClangModulesDeclVendorImpl::ForEachMacro( if (macro_info) { std::string macro_expansion = "#define "; - llvm::StringRef macro_identifier = mi->first->getName(); + llvm::StringRef macro_identifier = m.first->getName(); macro_expansion.append(macro_identifier.str()); { >From 10de023d9211b2b71be34cc1820195cf01edcf35 Mon Sep 17 00:00:00 2001 From: David Stone <[email protected]> Date: Thu, 14 May 2026 15:40:32 -0600 Subject: [PATCH 2/3] Fix `clang-tidy` identifier case warning --- .../clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp index e9cf61e5abf89..ca4a1044dd43d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp @@ -32,10 +32,10 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { return KeyValue.first->getName() == "SIGTERM" && KeyValue.first->hasMacroDefinition(); }; - const auto macros = PP->macros(); + const auto Macros = PP->macros(); const auto TryExpandAsInteger = [&](Preprocessor::macro_iterator It) -> std::optional<unsigned> { - if (It == macros.end()) + if (It == Macros.end()) return std::nullopt; const MacroInfo *MI = PP->getMacroInfo(It->first); const Token &T = MI->tokens().back(); @@ -56,7 +56,7 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) { return IntValue.getZExtValue(); }; - const auto SigtermMacro = llvm::find_if(macros, IsSigterm); + const auto SigtermMacro = llvm::find_if(Macros, IsSigterm); if (!SigtermValue && !(SigtermValue = TryExpandAsInteger(SigtermMacro))) return; >From 4ccf709e55a4f5c20804f8ce8f4c29e50882e831 Mon Sep 17 00:00:00 2001 From: David Stone <[email protected]> Date: Thu, 14 May 2026 15:46:00 -0600 Subject: [PATCH 3/3] Fix `clang-tidy` missing `const` warning --- .../clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp index e12a7cf883d46..6740f9a3f67fe 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp @@ -298,7 +298,7 @@ static void lengthExprHandle(const Expr *LengthExpr, // See whether we work with a macro. const StringRef LengthExprStr = exprToStr(LengthExpr, Result); - bool IsMacroDefinition = llvm::any_of(PP->macros(), [=](const auto &M) { + const bool IsMacroDefinition = llvm::any_of(PP->macros(), [=](const auto &M) { return M.first->getName() == LengthExprStr; }); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
