https://github.com/tJener created https://github.com/llvm/llvm-project/pull/233153
Starting in 91cdd35008e9, `NestedNameSpecifier` is a value-type handle, and `DynTypedNode` stores it by value. However, `hasPointerIdentity()` was not updated to return `false` for it, so users relying on the API to determine whether `DynTypedNode::get` returns a pointer to storage within `DynTypedNode` get the wrong answer for `NestedNameSpecifier`. This commit updates `hasPointerIdentity()` to return `false` for `NestedNameSpecifier`. Subsequently, `getMemoizationData()`'s behavior changes, returning a null pointer now. As a result, `DynTypedNode::operator<`, `opeartor==`, and `getHashValue` are also updated to handle `NestedNameSpecifier`. >From 523554a848f3b01fb8be1297e22c97d81eecfd56 Mon Sep 17 00:00:00 2001 From: Eric Li <[email protected]> Date: Sat, 17 Oct 2026 18:48:53 -0400 Subject: [PATCH] [clang][AST] Don't give `NestedNameSpecifier` pointer identity in `DynTypedNode` Starting in 91cdd35008e9, `NestedNameSpecifier` is a value-type handle, and `DynTypedNode` stores it by value. However, `hasPointerIdentity()` was not updated to return `false` for it, so users relying on the API to determine whether `DynTypedNode::get` returns a pointer to storage within `DynTypedNode` get the wrong answer for `NestedNameSpecifier`. This commit updates `hasPointerIdentity()` to return `false` for `NestedNameSpecifier`. Subsequently, `getMemoizationData()`'s behavior changes, returning a null pointer now. As a result, `DynTypedNode::operator<`, `opeartor==`, and `getHashValue` are also updated to handle `NestedNameSpecifier`. --- clang/include/clang/AST/ASTTypeTraits.h | 38 +++++++++++++------ clang/lib/AST/ASTTypeTraits.cpp | 2 +- clang/unittests/AST/ASTTypeTraitsTest.cpp | 24 ++++++++++++ .../ASTMatchers/Dynamic/ParserTest.cpp | 6 +-- .../ASTMatchers/Dynamic/RegistryTest.cpp | 5 ++- 5 files changed, 57 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/AST/ASTTypeTraits.h b/clang/include/clang/AST/ASTTypeTraits.h index 40d715390e757a..b075b80a20db7a 100644 --- a/clang/include/clang/AST/ASTTypeTraits.h +++ b/clang/include/clang/AST/ASTTypeTraits.h @@ -131,6 +131,7 @@ class ASTNodeKind { NKI_TemplateArgumentLoc, NKI_LambdaCapture, NKI_TemplateName, + NKI_NestedNameSpecifier, NKI_NestedNameSpecifierLoc, NKI_QualType, #define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc, @@ -139,7 +140,6 @@ class ASTNodeKind { NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc, NKI_CXXBaseSpecifier, NKI_CXXCtorInitializer, - NKI_NestedNameSpecifier, NKI_Decl, #define DECL(DERIVED, BASE) NKI_##DERIVED##Decl, #include "clang/AST/DeclNodes.inc" @@ -264,7 +264,7 @@ class DynTypedNode { /// convertible to \c T. /// /// For types that have identity via their pointer in the AST - /// (like \c Stmt, \c Decl, \c Type and \c NestedNameSpecifier) the returned + /// (like \c Stmt, \c Decl and \c Type) the returned /// pointer points to the referenced AST node. /// For other types (like \c QualType) the value is stored directly /// in the \c DynTypedNode, and the returned pointer points at @@ -310,7 +310,7 @@ class DynTypedNode { /// /// Supports comparison of nodes that support memoization. /// FIXME: Implement comparison for other node types (currently - /// only Stmt, Decl, Type and NestedNameSpecifier return memoization data). + /// only Stmt, Decl and Type return memoization data). bool operator<(const DynTypedNode &Other) const { if (!NodeKind.isSame(Other.NodeKind)) return NodeKind < Other.NodeKind; @@ -338,6 +338,10 @@ class DynTypedNode { NNSLB.getOpaqueData()); } + if (ASTNodeKind::getFromNodeKind<NestedNameSpecifier>().isSame(NodeKind)) + return getUnchecked<NestedNameSpecifier>().getAsVoidPointer() < + Other.getUnchecked<NestedNameSpecifier>().getAsVoidPointer(); + assert(getMemoizationData() && Other.getMemoizationData()); return getMemoizationData() < Other.getMemoizationData(); } @@ -358,6 +362,10 @@ class DynTypedNode { return getUnchecked<NestedNameSpecifierLoc>() == Other.getUnchecked<NestedNameSpecifierLoc>(); + if (ASTNodeKind::getFromNodeKind<NestedNameSpecifier>().isSame(NodeKind)) + return getUnchecked<NestedNameSpecifier>() == + Other.getUnchecked<NestedNameSpecifier>(); + assert(getMemoizationData() && Other.getMemoizationData()); return getMemoizationData() == Other.getMemoizationData(); } @@ -384,6 +392,11 @@ class DynTypedNode { NNSL.getOpaqueData()); } + if (ASTNodeKind::getFromNodeKind<NestedNameSpecifier>().isSame( + Val.NodeKind)) + return llvm::DenseMapInfo<NestedNameSpecifier>::getHashValue( + Val.getUnchecked<NestedNameSpecifier>()); + assert(Val.getMemoizationData()); return llvm::hash_value(Val.getMemoizationData()); } @@ -482,15 +495,16 @@ class DynTypedNode { /// Stores the data of the node. /// - /// Note that we can store \c Decls, \c Stmts, \c Types, - /// \c NestedNameSpecifiers and \c CXXCtorInitializer by pointer as they are - /// guaranteed to be unique pointers pointing to dedicated storage in the AST. - /// \c QualTypes, \c NestedNameSpecifierLocs, \c TypeLocs, - /// \c TemplateArguments and \c TemplateArgumentLocs on the other hand do not - /// have storage or unique pointers and thus need to be stored by value. - llvm::AlignedCharArrayUnion<const void *, TemplateArgument, - TemplateArgumentLoc, NestedNameSpecifierLoc, - QualType, TypeLoc, ObjCProtocolLoc> + /// Note that we can store \c Decls, \c Stmts, \c Types and + /// \c CXXCtorInitializer by pointer as they are guaranteed to be unique + /// pointers pointing to dedicated storage in the AST. + /// \c QualTypes, \c NestedNameSpecifiers, \c NestedNameSpecifierLocs, + /// \c TypeLocs, \c TemplateArguments and \c TemplateArgumentLocs on the other + /// hand do not have storage or unique pointers and thus need to be stored by + /// value. + llvm::AlignedCharArrayUnion< + const void *, TemplateArgument, TemplateArgumentLoc, NestedNameSpecifier, + NestedNameSpecifierLoc, QualType, TypeLoc, ObjCProtocolLoc> Storage; }; diff --git a/clang/lib/AST/ASTTypeTraits.cpp b/clang/lib/AST/ASTTypeTraits.cpp index 27bd21fc759400..9e3b7786892b2c 100644 --- a/clang/lib/AST/ASTTypeTraits.cpp +++ b/clang/lib/AST/ASTTypeTraits.cpp @@ -30,6 +30,7 @@ const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = { {NKI_None, "TemplateArgumentLoc"}, {NKI_None, "LambdaCapture"}, {NKI_None, "TemplateName"}, + {NKI_None, "NestedNameSpecifier"}, {NKI_None, "NestedNameSpecifierLoc"}, {NKI_None, "QualType"}, #define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"}, @@ -37,7 +38,6 @@ const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = { {NKI_None, "TypeLoc"}, {NKI_None, "CXXBaseSpecifier"}, {NKI_None, "CXXCtorInitializer"}, - {NKI_None, "NestedNameSpecifier"}, {NKI_None, "Decl"}, #define DECL(DERIVED, BASE) { NKI_##BASE, #DERIVED "Decl" }, #include "clang/AST/DeclNodes.inc" diff --git a/clang/unittests/AST/ASTTypeTraitsTest.cpp b/clang/unittests/AST/ASTTypeTraitsTest.cpp index b8f58b0cc6dd70..cb4adc9b8bf795 100644 --- a/clang/unittests/AST/ASTTypeTraitsTest.cpp +++ b/clang/unittests/AST/ASTTypeTraitsTest.cpp @@ -126,6 +126,7 @@ TEST(ASTNodeKind, ConstexprHasPointerIdentity) { EXPECT_TRUE(HasPointerIdentity<Stmt>); EXPECT_FALSE(HasPointerIdentity<TypeLoc>); EXPECT_FALSE(HasPointerIdentity<QualType>); + EXPECT_FALSE(HasPointerIdentity<NestedNameSpecifier>); EXPECT_FALSE(HasPointerIdentity<Foo>); constexpr bool DefaultConstructedHasPointerIdentity = @@ -243,6 +244,29 @@ TEST(DynTypedNode, QualType) { EXPECT_FALSE(Node < Node); } +TEST(DynTypedNode, NestedNameSpecifier) { + std::string code = R"cc(namespace a { struct b {}; } a::b x;)cc"; + auto AST = clang::tooling::buildASTFromCode(code); + auto matches = + match(nestedNameSpecifier(specifiesNamespace(hasName("a"))).bind("nns"), + AST->getASTContext()); + ASSERT_EQ(matches.size(), 1u); + + NestedNameSpecifier NNS = *matches[0].getNodeAs<NestedNameSpecifier>("nns"); + DynTypedNode Node = DynTypedNode::create(NNS); + DynTypedNode Copy = DynTypedNode::create(NNS); + EXPECT_EQ(Node.getMemoizationData(), nullptr); + EXPECT_TRUE(Node == Copy); + EXPECT_FALSE(Node < Copy); + EXPECT_FALSE(Copy < Node); + EXPECT_EQ(DynTypedNode::DenseMapInfo::getHashValue(Node), + DynTypedNode::DenseMapInfo::getHashValue(Copy)); + + DynTypedNode Global = DynTypedNode::create(NestedNameSpecifier::getGlobal()); + EXPECT_FALSE(Node == Global); + EXPECT_NE(Node < Global, Global < Node); +} + TEST(DynTypedNode, TypeLoc) { std::string code = R"cc(void example() { int abc; })cc"; auto AST = clang::tooling::buildASTFromCode(code); diff --git a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp index 88e141052ae015..4450387ca40064 100644 --- a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp +++ b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp @@ -545,9 +545,9 @@ TEST(ParserTest, CompletionRegistry) { Comps = Parser::completeExpression(Code, 6); ASSERT_EQ(1u, Comps.size()); EXPECT_EQ("Of(", Comps[0].TypedText); - EXPECT_EQ("Matcher<NestedNameSpecifierLoc|QualType|TypeLoc|...> " - "mapAnyOf(NestedNameSpecifierLoc|QualType|TypeLoc|" - "NestedNameSpecifier|Decl|Stmt|Type...)", + EXPECT_EQ("Matcher<NestedNameSpecifier|NestedNameSpecifierLoc|QualType|...> " + "mapAnyOf(NestedNameSpecifier|NestedNameSpecifierLoc|QualType|" + "TypeLoc|Decl|Stmt|Type...)", Comps[0].MatcherDecl); Code = "mapAnyOf(ifStmt)."; diff --git a/clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp b/clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp index be9bfa9d5986bb..cd772dcbdbcedb 100644 --- a/clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp +++ b/clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp @@ -497,8 +497,9 @@ TEST_F(RegistryTest, Completion) { // Polymorphic. EXPECT_TRUE(hasCompletion( Comps, "hasDescendant(", - "Matcher<NestedNameSpecifierLoc|QualType|TypeLoc|...> " - "hasDescendant(Matcher<NestedNameSpecifierLoc|QualType|TypeLoc|...>)")); + "Matcher<NestedNameSpecifier|NestedNameSpecifierLoc|QualType|...> " + "hasDescendant(Matcher<NestedNameSpecifier|NestedNameSpecifierLoc|" + "QualType|...>)")); CompVector WhileComps = getCompletions("whileStmt", 0); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
