njames93 added inline comments.
================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:313 + AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc), + std::string, MacroName) { // Verifies that the statement' beginning and ending are both expanded from ---------------- aaron.ballman wrote: > You mentioned that the change from `StringRef` to `std::string` was to avoid > lifetime issues while matching, but I'm wondering if you can expound on that > situation a bit more. I would have assumed that any memoization that involves > `StringRef` would be responsible for the lifetime issues rather than the > matchers themselves, but maybe I'm thinking about a different way you can hit > lifetime issues than you are. Take this as contrived, however using ASAN reports a heap-use-after-free for this code when isExpandedFromMacro takes a `StringRef` but works fine when using `std::string`. ```lang=c++ TEST(IsExpandedFromMacro, IsExpandedFromMacro_MatchesDecls) { auto Matcher = isExpandedFromMacro(std::string("MY_MACRO_OVERFLOW_SMALL_STRING_SIZE"));// <-Temporary string created and destroyed here. StringRef input = R"cc( #define MY_MACRO_OVERFLOW_SMALL_STRING_SIZE(a) int i = a; void Test() { MY_MACRO_OVERFLOW_SMALL_STRING_SIZE(4); } )cc"; EXPECT_TRUE(matches(input, varDecl(Matcher))); // <- heap-use-after-free detected down the callstack from here. } ``` Obviously this is very contrived to ensure asan detects the use-after-free and to ensure the temporary string is destroyed before the matcher. For these tests it doesn't look like a problem, but in other instances these matchers will outlive a string being passed to them ```lang=c++ void addVarFromMacro(ASTMatchFinder* Finder, std::string MacroName) { Finder->addMatcher(varDecl(isExpandedFromMacro(MacroName)), this); } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90303/new/ https://reviews.llvm.org/D90303 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits