Update `DynTypedNode` to support the conversion of `TypeLoc`s.

[jcking1034]

Summary

This provides better support for TypeLocs to allow TypeLoc-related
matchers to feature stricter typing and to avoid relying on the dynamic
casting of TypeLocs in matchers.

[ymandel]

Looks good! Please add a test to be sure it compiles/works correctly. Thanks!

clang/include/clang/AST/ASTTypeTraits.h

57

Here and below -- looks like clang format did this? I'd recommend you revert this line and others like it -- only change formatting on lines you've otherwise edited.

[sbenza]

clang/include/clang/AST/ASTTypeTraits.h

476

The create/get don't seem to match.
We are constructing a BaseT object in create(), so this static_cast to a derived type is UB?
That memory does not contain a T.

If we instead construct a T in create(), we should be able to read it as a BaseT* (assuming is_pointer_interconvertible_base_of<BaseT, T> is true)

[jcking1034]

Restore original formatting on unchanged lines.

[jcking1034]

Construct an object of the derived type, instead of the base type.

[jcking1034]

clang/include/clang/AST/ASTTypeTraits.h

476

Done, constructing a T instead of a BaseT seems to work!

[jcking1034]

Fix typing for create and add unit tests.

[ymandel]

clang/unittests/AST/ASTTypeTraitsTest.cpp

207

let's assert that matches.size() == 1 and then just use nodes[0].

[jcking1034]

Clean up unit tests.

[sbenza]

clang/unittests/AST/ASTTypeTraitsTest.cpp

212

I don't know what we are trying to check with this self equivalence.
Did you mean Node == tl?

225

We should check that we can do getNodeAs<PointerTypeLoc> and getNodeAs<TypeLoc>.
That is the goal of having the hierarchy in DynTypedNode.

[jcking1034]

Update unit test to better exercise getNodeAs

[jcking1034]

clang/unittests/AST/ASTTypeTraitsTest.cpp

212

These two EXPECTs are meant to ensure that the overloaded equality and less-than operators do function correctly when handling nodes of the TypeLoc family. In both cases, there is behavior specifically for these nodes, and I've made minor updates to both (changing isSame to isBaseOf). Additionally, this may give a little more confidence that template specialization functioned correctly. I will note that the QualType test performs a similar set of checks, presumably for similar reasoning.

[sbenza]

clang/unittests/AST/ASTTypeTraitsTest.cpp

234

Maybe also check

EXPECT_EQ(&tl, &ptl);

?

[jcking1034]

Include additional expectation in unit test

[jcking1034]

Fix pointer style

clang/include/clang/AST/ASTTypeTraits.h

 /// Use getFromNodeKind<T>() to construct them.
 class ASTNodeKind {
 public:
   /// Empty identifier. It matches nothing.
   ASTNodeKind() : KindId(NKI_None) {}
 
   /// Construct an identifier for T.
   template <class T>
   static ASTNodeKind getFromNodeKind() {
     return ASTNodeKind(KindToKindId<T>::Id);
   }
 
   /// \{
   /// Construct an identifier for the dynamic type of the node
   static ASTNodeKind getFromNode(const Decl &D);
   static ASTNodeKind getFromNode(const Stmt &S);
   static ASTNodeKind getFromNode(const Type &T);
+  static ASTNodeKind getFromNode(const TypeLoc &T);
   static ASTNodeKind getFromNode(const OMPClause &C);
   static ASTNodeKind getFromNode(const Attr &A);
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
   bool isSame(ASTNodeKind Other) const {
     return KindId != NKI_None && KindId == Other.KindId;
   }
private:
   /// Includes all possible base and derived kinds.
   enum NodeKindId {
     NKI_None,
     NKI_TemplateArgument,
     NKI_TemplateArgumentLoc,
     NKI_TemplateName,
     NKI_NestedNameSpecifierLoc,
     NKI_QualType,
+#define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc,
+#include "clang/AST/TypeLocNodes.def"
     NKI_TypeLoc,
     NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc,
     NKI_CXXBaseSpecifier,
     NKI_CXXCtorInitializer,
     NKI_NestedNameSpecifier,
     NKI_Decl,
 #define DECL(DERIVED, BASE) NKI_##DERIVED##Decl,
 #include "clang/AST/DeclNodes.inc"
#define KIND_TO_KIND_ID(Class)                                                 \
   };
 KIND_TO_KIND_ID(CXXCtorInitializer)
 KIND_TO_KIND_ID(TemplateArgument)
 KIND_TO_KIND_ID(TemplateArgumentLoc)
 KIND_TO_KIND_ID(TemplateName)
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
+#define TYPELOC(CLASS, PARENT) KIND_TO_KIND_ID(CLASS##TypeLoc)
+#include "clang/AST/TypeLocNodes.def"
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
 KIND_TO_KIND_ID(Type)
 KIND_TO_KIND_ID(OMPClause)
 KIND_TO_KIND_ID(Attr)
 KIND_TO_KIND_ID(CXXBaseSpecifier)
 #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
public:
   bool operator<(const DynTypedNode &Other) const {
     if (!NodeKind.isSame(Other.NodeKind))
       return NodeKind < Other.NodeKind;
 
     if (ASTNodeKind::getFromNodeKind<QualType>().isSame(NodeKind))
       return getUnchecked<QualType>().getAsOpaquePtr() <
              Other.getUnchecked<QualType>().getAsOpaquePtr();
 
-    if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(NodeKind)) {
+    if (ASTNodeKind::getFromNodeKind<TypeLoc>().isBaseOf(NodeKind)) {
       auto TLA = getUnchecked<TypeLoc>();
       auto TLB = Other.getUnchecked<TypeLoc>();
       return std::make_pair(TLA.getType().getAsOpaquePtr(),
                             TLA.getOpaqueData()) <
              std::make_pair(TLB.getType().getAsOpaquePtr(),
                             TLB.getOpaqueData());
     }
 
bool operator==(const DynTypedNode &Other) const {
     // If they contain the same node, their NodeKind must be the same.
     if (!NodeKind.isSame(Other.NodeKind))
       return false;
 
     // FIXME: Implement for other types.
     if (ASTNodeKind::getFromNodeKind<QualType>().isSame(NodeKind))
       return getUnchecked<QualType>() == Other.getUnchecked<QualType>();
 
-    if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(NodeKind))
+    if (ASTNodeKind::getFromNodeKind<TypeLoc>().isBaseOf(NodeKind))
       return getUnchecked<TypeLoc>() == Other.getUnchecked<TypeLoc>();
 
     if (ASTNodeKind::getFromNodeKind<NestedNameSpecifierLoc>().isSame(NodeKind))
       return getUnchecked<NestedNameSpecifierLoc>() ==
              Other.getUnchecked<NestedNameSpecifierLoc>();
 
     assert(getMemoizationData() && Other.getMemoizationData());
     return getMemoizationData() == Other.getMemoizationData();
struct DenseMapInfo {
     }
     static inline DynTypedNode getTombstoneKey() {
       DynTypedNode Node;
       Node.NodeKind = ASTNodeKind::DenseMapInfo::getTombstoneKey();
       return Node;
     }
     static unsigned getHashValue(const DynTypedNode &Val) {
       // FIXME: Add hashing support for the remaining types.
-      if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(Val.NodeKind)) {
+      if (ASTNodeKind::getFromNodeKind<TypeLoc>().isBaseOf(Val.NodeKind)) {
         auto TL = Val.getUnchecked<TypeLoc>();
         return llvm::hash_combine(TL.getType().getAsOpaquePtr(),
                                   TL.getOpaqueData());
       }
 
       if (ASTNodeKind::getFromNodeKind<NestedNameSpecifierLoc>().isSame(
               Val.NodeKind)) {
         auto NNSL = Val.getUnchecked<NestedNameSpecifierLoc>();
template <typename T> struct ValueConverter {
     static DynTypedNode create(const T &Node) {
       DynTypedNode Result;
       Result.NodeKind = ASTNodeKind::getFromNodeKind<T>();
       new (&Result.Storage) T(Node);
       return Result;
     }
   };
 
+  /// Converter that stores nodes by value. It must be possible to dynamically
+  /// cast the stored node within a type hierarchy without breaking (especially
+  /// through slicing).
+  template <typename T, typename BaseT,
+            typename = std::enable_if_t<(sizeof(T) == sizeof(BaseT))>>
+  struct DynCastValueConverter {
+    static const T *get(ASTNodeKind NodeKind, const void *Storage) {
+      if (ASTNodeKind::getFromNodeKind<T>().isBaseOf(NodeKind))
+        return &getUnchecked(NodeKind, Storage);
+      return nullptr;
+    }
+    static const T &getUnchecked(ASTNodeKind NodeKind, const void *Storage) {
+      assert(ASTNodeKind::getFromNodeKind<T>().isBaseOf(NodeKind));
+      return *static_cast<const T *>(reinterpret_cast<const BaseT *>(Storage));
+    }
+    static DynTypedNode create(const T &Node) {
+      DynTypedNode Result;
+      Result.NodeKind = ASTNodeKind::getFromNode(Node);
+      new (&Result.Storage) T(Node);
+      return Result;
+    }
+  };
+
   ASTNodeKind NodeKind;
 
   /// 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,

 struct DynTypedNode::BaseConverter<
     NestedNameSpecifierLoc,
     void> : public ValueConverter<NestedNameSpecifierLoc> {};
 
 template <>
 struct DynTypedNode::BaseConverter<QualType,
                                    void> : public ValueConverter<QualType> {};
 
-template <>
+template <typename T>
 struct DynTypedNode::BaseConverter<
-    TypeLoc, void> : public ValueConverter<TypeLoc> {};
+    T, std::enable_if_t<std::is_base_of<TypeLoc, T>::value>>
+    : public DynCastValueConverter<T, TypeLoc> {};
 
 template <>
 struct DynTypedNode::BaseConverter<CXXBaseSpecifier, void>
     : public PtrConverter<CXXBaseSpecifier> {};
 
 // The only operation we allow on unsupported types is \c get.
 // This allows to conveniently use \c DynTypedNode when having an arbitrary
 // AST node that is not supported, but prevents misuse - a user cannot create

clang/lib/AST/ASTTypeTraits.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/TypeLoc.h"
 
 using namespace clang;
 
 const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
     {NKI_None, "<None>"},
     {NKI_None, "TemplateArgument"},
     {NKI_None, "TemplateArgumentLoc"},
     {NKI_None, "TemplateName"},
     {NKI_None, "NestedNameSpecifierLoc"},
     {NKI_None, "QualType"},
+#define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"},
+#include "clang/AST/TypeLocNodes.def"
     {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"
     {NKI_None, "Stmt"},

 #define TYPE(Class, Base)                                                      \
     case Type::Class: return ASTNodeKind(NKI_##Class##Type);
 #define ABSTRACT_TYPE(Class, Base)
 #include "clang/AST/TypeNodes.inc"
   }
   llvm_unreachable("invalid type kind");
  }
 
+ ASTNodeKind ASTNodeKind::getFromNode(const TypeLoc &T) {
+   switch (T.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT)                                                 \
+  case TypeLoc::CLASS:                                                         \
+    return ASTNodeKind(NKI_##CLASS##TypeLoc);
+#include "clang/AST/TypeLocNodes.def"
+   }
+   llvm_unreachable("invalid typeloc kind");
+ }
+
 ASTNodeKind ASTNodeKind::getFromNode(const OMPClause &C) {
   switch (C.getClauseKind()) {
 #define GEN_CLANG_CLAUSE_CLASS
 #define CLAUSE_CLASS(Enum, Str, Class)                                         \
   case llvm::omp::Clause::Enum:                                                \
     return ASTNodeKind(NKI_##Class);
 #define CLAUSE_NO_CLASS(Enum, Str)                                             \
   case llvm::omp::Clause::Enum:                                                \

clang/unittests/AST/ASTTypeTraitsTest.cpp

 
 TEST(DynTypedNode, QualType) {
   QualType Q;
   DynTypedNode Node = DynTypedNode::create(Q);
   EXPECT_TRUE(Node == Node);
   EXPECT_FALSE(Node < Node);
 }
 
+TEST(DynTypedNode, TypeLoc) {
+  std::string code = R"cc(void example() { int abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto matches =
+      match(traverse(TK_AsIs,
+                     varDecl(hasName("abc"), hasTypeLoc(typeLoc().bind("tl")))),
+            AST->getASTContext());
+  EXPECT_EQ(matches.size(), 1u);
+
+  const auto &tl = *matches[0].getNodeAs<TypeLoc>("tl");
+  DynTypedNode Node = DynTypedNode::create(tl);
+  EXPECT_TRUE(Node == Node);
+  EXPECT_FALSE(Node < Node);
+}
+
+TEST(DynTypedNode, PointerTypeLoc) {
+  std::string code = R"cc(void example() { int *abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto matches =
+      match(traverse(TK_AsIs, varDecl(hasName("abc"),
+                                      hasTypeLoc(typeLoc().bind("ptl")))),
+            AST->getASTContext());
+  EXPECT_EQ(matches.size(), 1u);
+
+  const auto &tl = *matches[0].getNodeAs<TypeLoc>("ptl");
+  DynTypedNode TypeLocNode = DynTypedNode::create(tl);
+  EXPECT_TRUE(TypeLocNode == TypeLocNode);
+  EXPECT_FALSE(TypeLocNode < TypeLocNode);
+
+  const auto &ptl = *matches[0].getNodeAs<PointerTypeLoc>("ptl");
+  EXPECT_EQ(&tl, &ptl);
+  DynTypedNode PointerTypeLocNode = DynTypedNode::create(ptl);
+  EXPECT_TRUE(PointerTypeLocNode == PointerTypeLocNode);
+  EXPECT_FALSE(PointerTypeLocNode < PointerTypeLocNode);
+}
+
 } // namespace
 }  // namespace clang