Skip to content

Commit c9fbabf

Browse files
authored
[ASTMatcher] Fix redundant macro expansion checks in getExpansionLocOfMacro (#117143)
A performance issue was descibed in #114521 **Root Cause**: The function getExpansionLocOfMacro is responsible for finding the expansion location of macros. When dealing with macro parameters, it recursively calls itself to check the expansion of macro arguments. This recursive logic redundantly checks previous macro expansions, leading to significant performance degradation when macros are heavily nested. Solution **Modification**: Track already processed macros during recursion. Implementation Details: Introduced a data structure to record processed macros. Before each recursive call, check if the macro has already been processed to avoid redundant calculations. **Testing**: 1. refer to #114521 Finder->addMatcher(expr(isExpandedFromMacro("NULL")).bind("E"), this); run clang-tidy on freecad/src/Mod/Path/App/AreaPyImp.cpp clang-tidy src/Mod/Path/App/AreaPyImp.cpp -checks=-*,testchecker -p=build/compile_commands.json checker runs normally 2. check-clang-unit pass
1 parent 17cfd01 commit c9fbabf

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,8 @@ AST Matchers
971971
- Ensure ``hasName`` matches template specializations across inline namespaces,
972972
making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent.
973973

974+
- Improved the performance of the ``getExpansionLocOfMacro`` by tracking already processed macros during recursion.
975+
974976
- Add ``exportDecl`` matcher to match export declaration.
975977

976978
clang-format

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "clang/Basic/LLVM.h"
2222
#include "clang/Lex/Lexer.h"
2323
#include "llvm/ADT/ArrayRef.h"
24+
#include "llvm/ADT/DenseSet.h"
2425
#include "llvm/ADT/IntrusiveRefCntPtr.h"
2526
#include "llvm/ADT/SmallString.h"
2627
#include "llvm/ADT/SmallVector.h"
@@ -697,27 +698,42 @@ static bool isTokenAtLoc(const SourceManager &SM, const LangOptions &LangOpts,
697698
return !Invalid && Text == TokenText;
698699
}
699700

700-
std::optional<SourceLocation>
701-
getExpansionLocOfMacro(StringRef MacroName, SourceLocation Loc,
702-
const ASTContext &Context) {
701+
static std::optional<SourceLocation> getExpansionLocOfMacroRecursive(
702+
StringRef MacroName, SourceLocation Loc, const ASTContext &Context,
703+
llvm::DenseSet<SourceLocation> &CheckedLocations) {
703704
auto &SM = Context.getSourceManager();
704705
const LangOptions &LangOpts = Context.getLangOpts();
705706
while (Loc.isMacroID()) {
707+
if (CheckedLocations.count(Loc))
708+
return std::nullopt;
709+
CheckedLocations.insert(Loc);
706710
SrcMgr::ExpansionInfo Expansion =
707711
SM.getSLocEntry(SM.getFileID(Loc)).getExpansion();
708-
if (Expansion.isMacroArgExpansion())
712+
if (Expansion.isMacroArgExpansion()) {
709713
// Check macro argument for an expansion of the given macro. For example,
710714
// `F(G(3))`, where `MacroName` is `G`.
711-
if (std::optional<SourceLocation> ArgLoc = getExpansionLocOfMacro(
712-
MacroName, Expansion.getSpellingLoc(), Context))
715+
if (std::optional<SourceLocation> ArgLoc =
716+
getExpansionLocOfMacroRecursive(MacroName,
717+
Expansion.getSpellingLoc(),
718+
Context, CheckedLocations)) {
713719
return ArgLoc;
720+
}
721+
}
714722
Loc = Expansion.getExpansionLocStart();
715723
if (isTokenAtLoc(SM, LangOpts, MacroName, Loc))
716724
return Loc;
717725
}
718726
return std::nullopt;
719727
}
720728

729+
std::optional<SourceLocation>
730+
getExpansionLocOfMacro(StringRef MacroName, SourceLocation Loc,
731+
const ASTContext &Context) {
732+
llvm::DenseSet<SourceLocation> CheckedLocations;
733+
return getExpansionLocOfMacroRecursive(MacroName, Loc, Context,
734+
CheckedLocations);
735+
}
736+
721737
std::shared_ptr<llvm::Regex> createAndVerifyRegex(StringRef Regex,
722738
llvm::Regex::RegexFlags Flags,
723739
StringRef MatcherID) {

0 commit comments

Comments
 (0)