Skip some unnecessary type checks.

[sbenza]

Summary

Skip some unnecessary type checks wrt DynTypedNodes.
Add DynTypedNode::getUnchecked() to skip the runtime check when the type
is known.
Speed up DynTypedNode::operator== by using isSame() instead of
isBaseOf().
Skip the type check in MatcherInterface<T>::matches(). All calls come
from DynTypedMatcher::matches(), which already did the type check.
This change speeds up our clang-tidy benchmark by ~4%.

[klimek]

include/clang/AST/ASTTypeTraits.h

264 ↗

(On Diff #16786)

Why doesn't this change behavior? -> can we not get the same node with a different more or less specific type?

[sbenza]

include/clang/AST/ASTTypeTraits.h

264 ↗

(On Diff #16786)

It used to be that DynTypedNode::create() stored the static type of the node in NodeKind.
Recently we added ASTNodeKind::getFromNode() to get the dynamic type of the node and we store that on the DTN.
This was necessary for the RestrictKind optimizations.
Now that we store the exact node kind, we can compare them directly. This is also why we can drop the dyn_cast<> in favor of just cast<> in DynCastPtrConverter::get() below. The node kind check is enough.
Do you want a comment here?

[klimek]

lg with comment added :)

include/clang/AST/ASTTypeTraits.h

264 ↗

(On Diff #16786)

Yse please :)

[sbenza]

Closed by commit rL223134 (authored by @sbenza).

cfe/trunk/include/clang/AST/ASTTypeTraits.h

#include "clang/AST/TypeNodes.def"
   static bool isBaseOf(NodeKindId Base, NodeKindId Derived, unsigned *Distance);
 
   /// \brief Helper meta-function to convert a kind T to its enum value.
   ///
   /// This struct is specialized below for all known kinds.
   template <class T> struct KindToKindId {
     static const NodeKindId Id = NKI_None;
   };
+  template <class T>
+  struct KindToKindId<const T> : KindToKindId<T> {};
 
   /// \brief Per kind info.
   struct KindInfo {
     /// \brief The id of the parent kind, or None if it has no parent.
     NodeKindId ParentId;
     /// \brief Name of the kind.
     const char *Name;
   };
public:
   /// in the \c DynTypedNode, and the returned pointer points at
   /// the storage inside DynTypedNode. For those nodes, do not
   /// use the pointer outside the scope of the DynTypedNode.
   template <typename T>
   const T *get() const {
     return BaseConverter<T>::get(NodeKind, Storage.buffer);
   }
 
+  /// \brief Retrieve the stored node as type \c T.
+  ///
+  /// Similar to \c get(), but asserts that the type is what we are expecting.
+  template <typename T>
+  const T &getUnchecked() const {
+    return BaseConverter<T>::getUnchecked(NodeKind, Storage.buffer);
+  }
+
   ASTNodeKind getNodeKind() const { return NodeKind; }
 
   /// \brief Returns a pointer that identifies the stored AST node.
   ///
   /// Note that this is not supported by all AST nodes. For AST nodes
   /// that don't have a pointer-defined identity inside the AST, this
   /// method returns NULL.
   const void *getMemoizationData() const { return MemoizationData; }
public:
   /// Supports comparison of nodes that support memoization.
   /// FIXME: Implement comparsion for other node types (currently
   /// only Stmt, Decl, Type and NestedNameSpecifier return memoization data).
   bool operator<(const DynTypedNode &Other) const {
     assert(getMemoizationData() && Other.getMemoizationData());
     return getMemoizationData() < Other.getMemoizationData();
   }
   bool operator==(const DynTypedNode &Other) const {
-    if (!NodeKind.isBaseOf(Other.NodeKind) &&
-        !Other.NodeKind.isBaseOf(NodeKind))
+    // DynTypedNode::create() stores the exact kind of the node in NodeKind.
+    // 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>().isBaseOf(NodeKind)) {
-      return *get<QualType>() == *Other.get<QualType>();
-    }
+    if (ASTNodeKind::getFromNodeKind<QualType>().isSame(NodeKind))
+      return getUnchecked<QualType>() == Other.getUnchecked<QualType>();
+
     assert(getMemoizationData() && Other.getMemoizationData());
     return getMemoizationData() == Other.getMemoizationData();
   }
   bool operator!=(const DynTypedNode &Other) const {
     return !operator==(Other);
   }
   /// @}
 
 private:
   /// \brief Takes care of converting from and to \c T.
   template <typename T, typename EnablerT = void> struct BaseConverter;
 
   /// \brief Converter that uses dyn_cast<T> from a stored BaseT*.
   template <typename T, typename BaseT> struct DynCastPtrConverter {
     static const T *get(ASTNodeKind NodeKind, const char Storage[]) {
-      if (ASTNodeKind::getFromNodeKind<BaseT>().isBaseOf(NodeKind))
-        return dyn_cast<T>(*reinterpret_cast<BaseT *const *>(Storage));
+      if (ASTNodeKind::getFromNodeKind<T>().isBaseOf(NodeKind))
+        return cast<T>(*reinterpret_cast<BaseT *const *>(Storage));
       return nullptr;
     }
+    static const T &getUnchecked(ASTNodeKind NodeKind, const char Storage[]) {
+      assert(ASTNodeKind::getFromNodeKind<T>().isBaseOf(NodeKind));
+      return *cast<T>(*reinterpret_cast<BaseT *const *>(Storage));
+    }
     static DynTypedNode create(const BaseT &Node) {
       DynTypedNode Result;
       Result.NodeKind = ASTNodeKind::getFromNode(Node);
       Result.MemoizationData = &Node;
       new (Result.Storage.buffer) const BaseT * (&Node);
       return Result;
     }
   };
 
   /// \brief Converter that stores T* (by pointer).
   template <typename T> struct PtrConverter {
     static const T *get(ASTNodeKind NodeKind, const char Storage[]) {
       if (ASTNodeKind::getFromNodeKind<T>().isSame(NodeKind))
         return *reinterpret_cast<T *const *>(Storage);
       return nullptr;
     }
+    static const T &getUnchecked(ASTNodeKind NodeKind, const char Storage[]) {
+      assert(ASTNodeKind::getFromNodeKind<T>().isSame(NodeKind));
+      return **reinterpret_cast<T *const *>(Storage);
+    }
     static DynTypedNode create(const T &Node) {
       DynTypedNode Result;
       Result.NodeKind = ASTNodeKind::getFromNodeKind<T>();
       Result.MemoizationData = &Node;
       new (Result.Storage.buffer) const T * (&Node);
       return Result;
     }
   };
 
   /// \brief Converter that stores T (by value).
   template <typename T> struct ValueConverter {
     static const T *get(ASTNodeKind NodeKind, const char Storage[]) {
       if (ASTNodeKind::getFromNodeKind<T>().isSame(NodeKind))
         return reinterpret_cast<const T *>(Storage);
       return nullptr;
     }
+    static const T &getUnchecked(ASTNodeKind NodeKind, const char Storage[]) {
+      assert(ASTNodeKind::getFromNodeKind<T>().isSame(NodeKind));
+      return *reinterpret_cast<const T *>(Storage);
+    }
     static DynTypedNode create(const T &Node) {
       DynTypedNode Result;
       Result.NodeKind = ASTNodeKind::getFromNodeKind<T>();
       Result.MemoizationData = nullptr;
       new (Result.Storage.buffer) T(Node);
       return Result;
     }
   };

cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h

public:
   /// the AST via 'Finder'.
   virtual bool matches(const T &Node,
                        ASTMatchFinder *Finder,
                        BoundNodesTreeBuilder *Builder) const = 0;
 
   bool dynMatches(const ast_type_traits::DynTypedNode &DynNode,
                   ASTMatchFinder *Finder,
                   BoundNodesTreeBuilder *Builder) const override {
-    if (const T *Node = DynNode.get<T>()) {
-      return matches(*Node, Finder, Builder);
-    }
-    return false;
+    return matches(DynNode.getUnchecked<T>(), Finder, Builder);
   }
 };
 
 /// \brief Interface for matchers that only evaluate properties on a single
 /// node.
 template <typename T>
 class SingleNodeMatcherInterface : public MatcherInterface<T> {
 public: