Author: Ziqing Luo Date: 2026-06-25T13:54:01-07:00 New Revision: a18c09a05d6d6a950b1b7cd9c59a43fcee5cb442
URL: https://github.com/llvm/llvm-project/commit/a18c09a05d6d6a950b1b7cd9c59a43fcee5cb442 DIFF: https://github.com/llvm/llvm-project/commit/a18c09a05d6d6a950b1b7cd9c59a43fcee5cb442.diff LOG: [clang][index][USR] GenLoc prints file entry at most once, allow repeated offsets (#205430) `GenLoc` previously printed the source location at most once per USR, gated by a member flag toggled on the first call. During the recursive visit, if both an outer and an inner decl needed to print the location, only the outer one was printed. When the outer decl did not need the offset, no offset was ever printed. For example, the USR of `Holder<decltype([]{})>::method` depends on the location of the type of the lambda but the outer decl prints the file entry only, which disables offset printing. Change the logic so the file-entry part of the location is printed at most once (it must be identical), while offsets of sub-decl locations may be printed multiple times. rdar://180654884 --------- Co-authored-by: Balázs Benics <[email protected]> Added: Modified: clang/lib/UnifiedSymbolResolution/USRGeneration.cpp clang/test/Analysis/Scalable/PointerFlow/entity-name-no-conflict.cpp clang/unittests/Index/IndexTests.cpp Removed: ################################################################################ diff --git a/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp b/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp index c4788dd6917f2..5f689ad32159b 100644 --- a/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp +++ b/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp @@ -24,6 +24,28 @@ using namespace clang::index; // USR generation. //===----------------------------------------------------------------------===// +/// Print only the offset part of \p Loc +/// \returns true on error. +static bool printLocOffset(llvm::raw_ostream &OS, SourceLocation Loc, + const SourceManager &SM) { + if (Loc.isInvalid()) + return true; + if (SM.getExpansionLoc(Loc) == SM.getSpellingLoc(Loc)) { + // Use the offest into the FileID to represent the location. Using + // a line/column can cause us to look back at the original source file, + // which is expensive. + OS << '@' << SM.getDecomposedLoc(Loc).second; + return false; + } + // In case expansion and spelling locations diff er, we need both of them to + // make the USR distinguishable: + OS << '@' << SM.getDecomposedLoc(SM.getExpansionLoc(Loc)).second; + OS << '@' << SM.getDecomposedLoc(SM.getSpellingLoc(Loc)).second; + return false; +} + +/// Print \p Loc including both the file and offset, if \p IncludeOffset is +/// true. Print only the file of \p Loc otherwise. /// \returns true on error. static bool printLoc(llvm::raw_ostream &OS, SourceLocation Loc, const SourceManager &SM, bool IncludeOffset) { @@ -43,7 +65,7 @@ static bool printLoc(llvm::raw_ostream &OS, SourceLocation Loc, // Use the offest into the FileID to represent the location. Using // a line/column can cause us to look back at the original source file, // which is expensive. - OS << '@' << Decomposed.second; + printLocOffset(OS, Loc, SM); } return false; } @@ -64,7 +86,11 @@ class USRGenerator : public ConstDeclVisitor<USRGenerator> { ASTContext *Context; const LangOptions &LangOpts; bool IgnoreResults = false; - bool generatedLoc = false; + // The flag below ensures that, when a source location needs to be printed, + // the filename is printed at most once during the recursive visit. The + // offset part may be printed multiple times, since sub-decls along the + // visit may each need a distinct location to disambiguate them. + bool GeneratedFilename = false; llvm::DenseMap<const Type *, unsigned> TypeSubstitutions; @@ -636,23 +662,28 @@ void USRGenerator::GenExtSymbolContainer(const NamedDecl *D) { } bool USRGenerator::GenLoc(const Decl *D, bool IncludeOffset) { - if (generatedLoc) - return IgnoreResults; - generatedLoc = true; - // Guard against null declarations in invalid code. if (!D) { IgnoreResults = true; return true; } + // Do nothing if no need to print the offset nor the filename: + if (!IncludeOffset && GeneratedFilename) + return IgnoreResults; + + bool PrintErr = false; // Use the location of canonical decl. D = D->getCanonicalDecl(); - - IgnoreResults = - IgnoreResults || printLoc(Out, D->getBeginLoc(), - Context->getSourceManager(), IncludeOffset); - + if (!GeneratedFilename) { + GeneratedFilename = true; + PrintErr = printLoc(Out, D->getBeginLoc(), Context->getSourceManager(), + IncludeOffset); + } else { + PrintErr = + printLocOffset(Out, D->getBeginLoc(), Context->getSourceManager()); + } + IgnoreResults = IgnoreResults || PrintErr; return IgnoreResults; } diff --git a/clang/test/Analysis/Scalable/PointerFlow/entity-name-no-conflict.cpp b/clang/test/Analysis/Scalable/PointerFlow/entity-name-no-conflict.cpp index e190ee264cb7d..c045e732ea6c1 100644 --- a/clang/test/Analysis/Scalable/PointerFlow/entity-name-no-conflict.cpp +++ b/clang/test/Analysis/Scalable/PointerFlow/entity-name-no-conflict.cpp @@ -4,7 +4,7 @@ // RUN: --ssaf-extract-summaries=PointerFlow \ // RUN: --ssaf-tu-summary-file=%t/tu.summary.json \ // RUN: --ssaf-compilation-unit-id="tu-1" \ -// RUN: -mllvm -debug-only=ssaf-analyses 2>&1 | FileCheck %s +// RUN: -mllvm -debug-only=ssaf-analyses 2>&1 | FileCheck %s --allow-empty // The two `Holder<decltype([]{})>` instantiations are distinct types @@ -12,8 +12,7 @@ // currently fails to distinguish them. -// CHECK: dropping duplicate PointerFlow summary -// FIXME: change to CHECK-NOT once the bug gets fixed +// CHECK-NOT: dropping duplicate PointerFlow summary template <class T> struct Holder { diff --git a/clang/unittests/Index/IndexTests.cpp b/clang/unittests/Index/IndexTests.cpp index 6df4b577d98a0..e20764e4fb9d9 100644 --- a/clang/unittests/Index/IndexTests.cpp +++ b/clang/unittests/Index/IndexTests.cpp @@ -9,6 +9,10 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/CompilerInstance.h" @@ -18,6 +22,7 @@ #include "clang/Index/IndexingAction.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/Tooling.h" +#include "clang/UnifiedSymbolResolution/USRGeneration.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" @@ -451,6 +456,86 @@ TEST(IndexTest, ReadWriteRoles) { WrittenAt(Position(6, 17)))))); } +// Two `Holder<decltype([]{})>` instantiations have distinct closure types, +// therefore they should have distinct USRs. +TEST(USRTest, NestedLambdaTagsInTemplateArg) { + auto AST = tooling::buildASTFromCodeWithArgs( + R"cpp( + template <class T> struct Holder { void reset(T*) {} }; + void caller() { + Holder<decltype([]{})>().reset(nullptr); + Holder<decltype([]{})>().reset(nullptr); + } + )cpp", + {"-std=c++20"}); + ASSERT_TRUE(AST); + + using namespace ast_matchers; + auto Matches = match( + cxxMemberCallExpr(callee(cxxMethodDecl(ofClass( + classTemplateSpecializationDecl(hasName("Holder")).bind("spec"))))), + AST->getASTContext()); + ASSERT_EQ(Matches.size(), 2u); + + llvm::SmallString<128> U0, U1; + ASSERT_FALSE(generateUSRForDecl( + Matches[0].getNodeAs<ClassTemplateSpecializationDecl>("spec"), U0)); + ASSERT_FALSE(generateUSRForDecl( + Matches[1].getNodeAs<ClassTemplateSpecializationDecl>("spec"), U1)); + EXPECT_NE(U0, U1) << "U0=" << U0.str() << " U1=" << U1.str(); +} + +// A variant of `NestedLambdaTagsInTemplateArg` that tests when the template +// specializations have identical expansion location but distinct spelling +// location. +TEST(USRTest, NestedLambdaTagsInTemplateArgWithSameSpellingDistinctExpansion) { + auto AST = tooling::buildASTFromCodeWithArgs( + R"cpp( + #define MY_SPEC Holder<decltype([]{})>().reset(nullptr); \ + Holder<decltype([]{})>().reset(nullptr) + template <class T> struct Holder { void reset(T*) {} }; + void caller() { + MY_SPEC; + } + )cpp", + {"-std=c++20"}); + ASSERT_TRUE(AST); + + using namespace ast_matchers; + auto Matches = + match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass( + classTemplateSpecializationDecl( + hasName("Holder"), + hasTemplateArgument( + 0, refersToType(hasDeclaration( + cxxRecordDecl(isLambda()).bind("closure"))))) + .bind("spec"))))), + AST->getASTContext()); + ASSERT_EQ(Matches.size(), 2u); + + const auto *CTSD1 = + Matches[0].getNodeAs<ClassTemplateSpecializationDecl>("spec"); + const auto *CTSD2 = + Matches[1].getNodeAs<ClassTemplateSpecializationDecl>("spec"); + const auto *CRD1 = Matches[0].getNodeAs<CXXRecordDecl>("closure"); + const auto *CRD2 = Matches[1].getNodeAs<CXXRecordDecl>("closure"); + + // First, check that the two specializations have distinct USR names: + llvm::SmallString<128> U0, U1; + ASSERT_FALSE(generateUSRForDecl(CTSD1, U0)); + ASSERT_FALSE(generateUSRForDecl(CTSD2, U1)); + ASSERT_NE(U0, U1) << "U0=" << U0.str() << " U1=" << U1.str(); + + // Second, check that the two lambda types have distinct spellingLoc offsets + // but identical expansionLoc offsets: + const SourceManager &SM = AST->getASTContext().getSourceManager(); + + ASSERT_NE(SM.getDecomposedLoc(SM.getSpellingLoc(CRD1->getBeginLoc())), + SM.getDecomposedLoc(SM.getSpellingLoc(CRD2->getBeginLoc()))); + ASSERT_EQ(SM.getDecomposedLoc(SM.getExpansionLoc(CRD1->getBeginLoc())), + SM.getDecomposedLoc(SM.getExpansionLoc(CRD2->getBeginLoc()))); +} + } // namespace } // namespace index } // namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
