[clang] [clang][AST] Don't give `NestedNameSpecifier` pointer identity in `DynTypedNode` (PR #233153)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 17 18:04:18 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Eric Li (tJener)
<details>
<summary>Changes</summary>
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`.
---
Full diff: https://github.com/llvm/llvm-project/pull/233153.diff
5 Files Affected:
- (modified) clang/include/clang/AST/ASTTypeTraits.h (+26-12)
- (modified) clang/lib/AST/ASTTypeTraits.cpp (+1-1)
- (modified) clang/unittests/AST/ASTTypeTraitsTest.cpp (+24)
- (modified) clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp (+3-3)
- (modified) clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp (+3-2)
``````````diff
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);
``````````
</details>
https://github.com/llvm/llvm-project/pull/233153
More information about the cfe-commits
mailing list