[ASTMatchers][NFC] Remove runtime checks where compile time is sufficient

[njames93]

Summary

Some polymorphic matchers use dyn_cast in cases where we know the type from the TemplateParameter.
Using type_traits we can remove the need for dyn_cast and let the template instantiation handle everything.

[aaron.ballman]

I'm not opposed, but I don't know that this is really an improvement. We've gone from a pretty simple code pattern to needing to spell out the type twice with a type_trait, and all that we save is a call to isa<> within the casting operation. While I agree this is strictly executing less code, it's morally the same as the usual antipattern of "isa followed by cast should be a dyn_cast". Is there evidence that this is a big performance win?

[njames93]

In D102369#2756493, @aaron.ballman wrote:

I'm not opposed, but I don't know that this is really an improvement. We've gone from a pretty simple code pattern to needing to spell out the type twice with a type_trait, and all that we save is a call to isa<> within the casting operation. While I agree this is strictly executing less code, it's morally the same as the usual antipattern of "isa followed by cast should be a dyn_cast". Is there evidence that this is a big performance win?

To be honest, it would be much nicer if we could specialise isa to return false if the From type is not a base class of the To type. However I'm not sure if that would break anything else.

It's all part of the "Don't do at runtime whats known at compile time" idiom. Any performance change would be negligible in this case.

[aaron.ballman]

In D102369#2756516, @njames93 wrote:

In D102369#2756493, @aaron.ballman wrote:

I'm not opposed, but I don't know that this is really an improvement. We've gone from a pretty simple code pattern to needing to spell out the type twice with a type_trait, and all that we save is a call to isa<> within the casting operation. While I agree this is strictly executing less code, it's morally the same as the usual antipattern of "isa followed by cast should be a dyn_cast". Is there evidence that this is a big performance win?

To be honest, it would be much nicer if we could specialise isa to return false if the From type is not a base class of the To type. However I'm not sure if that would break anything else.

That might be interesting to explore.

It's all part of the "Don't do at runtime whats known at compile time" idiom. Any performance change would be negligible in this case.

If that's the sole reason for the change, I'm not convinced it carries its weight. It's more code, it's more complicated to read, violates the DRY principle, and has negligible performance benefit. However, perhaps @klimek has different opinions as code owner.

clang/include/clang/ASTMatchers/ASTMatchers.h

 /// \endcode
 ///
 /// Usable as: Matcher<CXXRecordDecl>, Matcher<ObjCInterfaceDecl>
 AST_POLYMORPHIC_MATCHER_P(
     isDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     internal::Matcher<NamedDecl>, Base) {
   // Check if the node is a C++ struct/union/class.
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Finder->classIsDerivedFrom(RD, Base, Builder, /*Directly=*/false);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Finder->classIsDerivedFrom(cast<CXXRecordDecl>(&Node), Base, Builder,
+                                      /*Directly=*/false);
 
   // The node must be an Objective-C class.
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Finder->objcClassIsDerivedFrom(InterfaceDecl, Base, Builder,
                                         /*Directly=*/false);
 }
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     isDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     std::string, BaseName, 1) {
   if (BaseName.empty())
     return false;
 
   const auto M = isDerivedFrom(hasName(BaseName));
 
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Matcher<CXXRecordDecl>(M).matches(*RD, Finder, Builder);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Matcher<CXXRecordDecl>(M).matches(cast<CXXRecordDecl>(Node), Finder,
+                                             Builder);
 
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Matcher<ObjCInterfaceDecl>(M).matches(*InterfaceDecl, Finder, Builder);
 }
 
 /// Matches C++ classes that have a direct or indirect base matching \p
 /// BaseSpecMatcher.
 ///

 /// Similar to \c isDerivedFrom(), but also matches classes that directly
 /// match \c Base.
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     isSameOrDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     internal::Matcher<NamedDecl>, Base, 0) {
   const auto M = anyOf(Base, isDerivedFrom(Base));
 
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Matcher<CXXRecordDecl>(M).matches(*RD, Finder, Builder);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Matcher<CXXRecordDecl>(M).matches(cast<CXXRecordDecl>(Node), Finder,
+                                             Builder);
 
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Matcher<ObjCInterfaceDecl>(M).matches(*InterfaceDecl, Finder, Builder);
 }
 
 /// Overloaded method as shortcut for
 /// \c isSameOrDerivedFrom(hasName(...)).
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     isSameOrDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     std::string, BaseName, 1) {
   if (BaseName.empty())
     return false;
 
   const auto M = isSameOrDerivedFrom(hasName(BaseName));
 
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Matcher<CXXRecordDecl>(M).matches(*RD, Finder, Builder);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Matcher<CXXRecordDecl>(M).matches(cast<CXXRecordDecl>(Node), Finder,
+                                             Builder);
 
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Matcher<ObjCInterfaceDecl>(M).matches(*InterfaceDecl, Finder, Builder);
 }
 
 /// Matches C++ or Objective-C classes that are directly derived from a class
 /// matching \c Base.
 ///

 ///   typedef Foo X;
 ///   class Bar : public Foo {};  // derived from a type that X is a typedef of
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     isDirectlyDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     internal::Matcher<NamedDecl>, Base, 0) {
   // Check if the node is a C++ struct/union/class.
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Finder->classIsDerivedFrom(RD, Base, Builder, /*Directly=*/true);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Finder->classIsDerivedFrom(cast<CXXRecordDecl>(&Node), Base, Builder,
+                                      /*Directly=*/true);
 
   // The node must be an Objective-C class.
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Finder->objcClassIsDerivedFrom(InterfaceDecl, Base, Builder,
                                         /*Directly=*/true);
 }
 
 /// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     isDirectlyDerivedFrom,
     AST_POLYMORPHIC_SUPPORTED_TYPES(CXXRecordDecl, ObjCInterfaceDecl),
     std::string, BaseName, 1) {
   if (BaseName.empty())
     return false;
   const auto M = isDirectlyDerivedFrom(hasName(BaseName));
 
-  if (const auto *RD = dyn_cast<CXXRecordDecl>(&Node))
-    return Matcher<CXXRecordDecl>(M).matches(*RD, Finder, Builder);
+  if (std::is_convertible<NodeType *, CXXRecordDecl *>::value)
+    return Matcher<CXXRecordDecl>(M).matches(cast<CXXRecordDecl>(Node), Finder,
+                                             Builder);
 
   const auto *InterfaceDecl = cast<ObjCInterfaceDecl>(&Node);
   return Matcher<ObjCInterfaceDecl>(M).matches(*InterfaceDecl, Finder, Builder);
 }
 /// Matches the first method of a class or struct that satisfies \c
 /// InnerMatcher.
 ///
 /// Given:

 /// memberExpr(hasObjectExpression(hasType(pointsTo(
 //      cxxRecordDecl(hasName("X"))))))
 ///   matches `m` (aka. `this->m`), but not `x.m`.
 AST_POLYMORPHIC_MATCHER_P(
     hasObjectExpression,
     AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
                                     CXXDependentScopeMemberExpr),
     internal::Matcher<Expr>, InnerMatcher) {
-  if (const auto *E = dyn_cast<UnresolvedMemberExpr>(&Node))
-    if (E->isImplicitAccess())
+  if (std::is_convertible<NodeType *, UnresolvedMemberExpr *>::value) {
+    if (cast<UnresolvedMemberExpr>(&Node)->isImplicitAccess())
       return false;
-  if (const auto *E = dyn_cast<CXXDependentScopeMemberExpr>(&Node))
-    if (E->isImplicitAccess())
+  } else if (std::is_convertible<NodeType *,
+                                 CXXDependentScopeMemberExpr *>::value) {
+    if (cast<CXXDependentScopeMemberExpr>(&Node)->isImplicitAccess())
       return false;
+  }
   return InnerMatcher.matches(*Node.getBase(), Finder, Builder);
 }
 
 /// Matches any using shadow declaration.
 ///
 /// Given
 /// \code
 ///   namespace X { void b(); }

 /// \endcode
 /// functionDecl(isInline()) will match ::f().
 /// namespaceDecl(isInline()) will match n::m.
 AST_POLYMORPHIC_MATCHER(isInline,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
                                                         FunctionDecl)) {
   // This is required because the spelling of the function used to determine
   // whether inline is specified or not differs between the polymorphic types.
-  if (const auto *FD = dyn_cast<FunctionDecl>(&Node))
-    return FD->isInlineSpecified();
-  else if (const auto *NSD = dyn_cast<NamespaceDecl>(&Node))
-    return NSD->isInline();
+  if (std::is_convertible<NodeType *, FunctionDecl *>::value)
+    return cast<FunctionDecl>(&Node)->isInlineSpecified();
+  else if (std::is_convertible<NodeType *, NamespaceDecl *>::value)
+    return cast<NamespaceDecl>(&Node)->isInline();
   llvm_unreachable("Not a valid polymorphic type");
 }
 
 /// Matches anonymous namespace declarations.
 ///
 /// Given
 /// \code
 ///   namespace n {