llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Ziqing Luo (ziqingluo-90)
<details>
<summary>Changes</summary>
`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.
---
Full diff: https://github.com/llvm/llvm-project/pull/205430.diff
2 Files Affected:
- (modified) clang/lib/UnifiedSymbolResolution/USRGeneration.cpp (+35-10)
- (modified) clang/unittests/Index/IndexTests.cpp (+34)
``````````diff
diff --git a/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp
b/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp
index c4788dd6917f2..7c254e0c39c32 100644
--- a/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp
+++ b/clang/lib/UnifiedSymbolResolution/USRGeneration.cpp
@@ -24,6 +24,22 @@ 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;
+ Loc = SM.getExpansionLoc(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;
+}
+
+/// 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) {
@@ -64,7 +80,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 file entry is printed at most once during the recursive visit. The
+ // offset part may be printed multiple times, since sub-decls along the
+ // path may each need a distinct location to disambiguate them.
+ bool GeneratedFileEntry = false;
llvm::DenseMap<const Type *, unsigned> TypeSubstitutions;
@@ -636,23 +656,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 file entry has been printed and no need to print the offset.
+ if (GeneratedFileEntry && !IncludeOffset)
+ return IgnoreResults;
+
+ bool PrintErr = false;
// Use the location of canonical decl.
D = D->getCanonicalDecl();
-
- IgnoreResults =
- IgnoreResults || printLoc(Out, D->getBeginLoc(),
- Context->getSourceManager(), IncludeOffset);
-
+ if (!GeneratedFileEntry) {
+ GeneratedFileEntry = 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/unittests/Index/IndexTests.cpp
b/clang/unittests/Index/IndexTests.cpp
index 6df4b577d98a0..90d47657fc5cc 100644
--- a/clang/unittests/Index/IndexTests.cpp
+++ b/clang/unittests/Index/IndexTests.cpp
@@ -9,6 +9,7 @@
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -18,6 +19,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 +453,38 @@ TEST(IndexTest, ReadWriteRoles) {
WrittenAt(Position(6, 17))))));
}
+// Two `Holder<decltype([]{})>` instantiations have distinct closure types,
+// therefore they should have distinct USRs.
+TEST(USRTest, NestedLambdaTagsInTemplateArgGetDistinctUSRs) {
+ 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);
+
+ struct Visitor : RecursiveASTVisitor<Visitor> {
+ std::vector<const ClassTemplateSpecializationDecl *> Specs;
+ bool VisitCXXMemberCallExpr(CXXMemberCallExpr *Call) {
+ if (auto *M = Call->getMethodDecl())
+ if (auto *S =
dyn_cast<ClassTemplateSpecializationDecl>(M->getParent()))
+ Specs.push_back(S);
+ return true;
+ }
+ } V;
+ V.TraverseAST(AST->getASTContext());
+ ASSERT_EQ(V.Specs.size(), 2u);
+
+ llvm::SmallString<128> U0, U1;
+ ASSERT_FALSE(generateUSRForDecl(V.Specs[0], U0));
+ ASSERT_FALSE(generateUSRForDecl(V.Specs[1], U1));
+ EXPECT_NE(U0, U1) << "U0=" << U0.str() << " U1=" << U1.str();
+}
+
} // namespace
} // namespace index
} // namespace clang
``````````
</details>
https://github.com/llvm/llvm-project/pull/205430
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
