[ASTMatchers] Make isNoThrow and hasDynamicExceptionSpec polymorphic for use with both functionDecl and functionProtoType

[hintonda]

Summary

Make isNoThrow and hasDynamicExceptionSpec polymorphic so they can be used with both functionDecl and functionPrototype matchers.

[hintonda]

Ping...

[aaron.ballman]

Sorry for the delayed response, this review got lost in my inbox!

This change will break existing out-of-tree users and would be a regression. Instead, the matcher should become a polymorphic matcher that accepts a FunctionDecl as well as a FunctionType. Check out the matcher for isConstexpr() to see how to create a polymorphic matcher. Note that you may need to use a trick like the one used in hasAnySubstatement() to handle the differences between getting the exception spec from a FunctionDecl and a FunctionType from the same matcher.

Also, can you update isNoThrow() at the same time? It has the similar behavior of currently working only on a FunctionDecl and not a FunctionType.

Finally, I'm wondering whether we want this to work on a FunctionType or a FunctionProtoType. Only functions with prototypes can have an exception specification, so putting the functionality on FunctionProtoType keeps it closer to the AST representation. However, this also may make it slightly more awkward to use in practice. Thoughts?

[hintonda]

This marcher was recently added, and has never been in a release. Specifically, it was added by me in support of a checker that has now been abandoned in lieu of a better approach -- the new approach requires this change -- so I doubt it would break anything.

That said, I will look into polymorphic matchers if that is the preferred solution.

[aaron.ballman]

In D20687#449312, @hintonda wrote:

This marcher was recently added, and has never been in a release. Specifically, it was added by me in support of a checker that has now been abandoned in lieu of a better approach -- the new approach requires this change -- so I doubt it would break anything.

That's good to know, thanks!

That said, I will look into polymorphic matchers if that is the preferred solution.

Thank you -- I think that is still the right approach to take. We try to keep the matchers as close to the AST representation as possible, and this one (plus isNoThrow()) slipped through the cracks (in a useful way).

[hintonda]

Update narrowing matchers, isNoThrow and hasDynamicExceptionSpec, to
work with both functionDecl and functionProtoType matchers. Update
tests and regenerate documentation.

Since FunctionDecl and FunctionProtoType are not polymorphically
related, an overloaded helper function, getFunctionProtoType(), was
added that takes either a FunctionDecl node or FunctionProtoType node
and returns a FunctionProtoType pointer. This is based on the
getUnderlyingType() implementation.

[aaron.ballman]

This is looking good, aside from a few small nits. The only question I have remaining is whether this should be on FunctionProtoType or FunctionType? I think being on FunctionProtoType (how you have it) is preferable. @sbenza or @klimek, do you agree?

include/clang/ASTMatchers/ASTMatchers.h

3256

Since the type isn't explicitly spelled out in the initialization, probably should use const FunctionProtoType * instead of const auto *.

3274

Same here.

[hintonda]

Use FunctionProtoType instead of auto for clarity.

[aaron.ballman]

LGTM, thank you for working on this!

[hintonda]

Thanks Aaron. If you could commit for me, I'd appreciate it. Thanks again...

[aaron.ballman]

Committed in r272028.

docs/LibASTMatchersReference.html

 Example matches true (matcher = cxxBoolLiteral(equals(true)))
   true
 
 Usable as: Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CharacterLiteral.html">CharacterLiteral</a>&gt;, Matcher&lt;CXXBoolLiteral&gt;,
            Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FloatingLiteral.html">FloatingLiteral</a>&gt;, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1IntegerLiteral.html">IntegerLiteral</a>&gt;
 </pre></td></tr>
 
 
-<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('hasDynamicExceptionSpec0')"><a name="hasDynamicExceptionSpec0Anchor">hasDynamicExceptionSpec</a></td><td></td></tr>
-<tr><td colspan="4" class="doc" id="hasDynamicExceptionSpec0"><pre>Matches functions that have a dynamic exception specification.
-
-Given:
-  void f();
-  void g() noexcept;
-  void h() noexcept(true);
-  void i() noexcept(false);
-  void j() throw();
-  void k() throw(int);
-  void l() throw(...);
-functionDecl(hasDynamicExceptionSpec())
-  matches the declarations of j, k, and l, but not f, g, h, or i.
-</pre></td></tr>
-
-
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('hasOverloadedOperatorName0')"><a name="hasOverloadedOperatorName0Anchor">hasOverloadedOperatorName</a></td><td>StringRef Name</td></tr>
 <tr><td colspan="4" class="doc" id="hasOverloadedOperatorName0"><pre>Matches overloaded operator names.
 
 Matches overloaded operator names specified in strings without the
 "operator" prefix: e.g. "&lt;&lt;".
 
 Given:
   class A { int operator*(); };
functionDecl(parameterCountIs(2))
   matches void g(int i, int j) {}
 functionProtoType(parameterCountIs(2))
   matches void h(int i, int j)
 functionProtoType(parameterCountIs(3))
   matches void k(int x, int y, int z, ...);
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionType.html">FunctionType</a>&gt;</td><td class="name" onclick="toggle('hasDynamicExceptionSpec0')"><a name="hasDynamicExceptionSpec0Anchor">hasDynamicExceptionSpec</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="hasDynamicExceptionSpec0"><pre>Matches functions that have a dynamic exception specification.
+
+Given:
+  void f();
+  void g() noexcept;
+  void h() noexcept(true);
+  void i() noexcept(false);
+  void j() throw();
+  void k() throw(int);
+  void l() throw(...);
+functionType(hasDynamicExceptionSpec())
+  matches the declarations of j, k, and l, but not f, g, h, or i.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1IntegerLiteral.html">IntegerLiteral</a>&gt;</td><td class="name" onclick="toggle('equals0')"><a name="equals0Anchor">equals</a></td><td>ValueT  Value</td></tr>
 <tr><td colspan="4" class="doc" id="equals0"><pre>Matches literals that are equal to the given value.
 
 Example matches true (matcher = cxxBoolLiteral(equals(true)))
   true
 
 Usable as: Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CharacterLiteral.html">CharacterLiteral</a>&gt;, Matcher&lt;CXXBoolLiteral&gt;,
            Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FloatingLiteral.html">FloatingLiteral</a>&gt;, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1IntegerLiteral.html">IntegerLiteral</a>&gt;

include/clang/ASTMatchers/ASTMatchers.h

 ///   void f();
 ///   void g() noexcept;
 ///   void h() noexcept(true);
 ///   void i() noexcept(false);
 ///   void j() throw();
 ///   void k() throw(int);
 ///   void l() throw(...);
 /// \endcode
-/// functionDecl(hasDynamicExceptionSpec())
+/// functionType(hasDynamicExceptionSpec())
 ///   matches the declarations of j, k, and l, but not f, g, h, or i.
-AST_MATCHER(FunctionDecl, hasDynamicExceptionSpec) {
-  if (const auto *FnTy = Node.getType()->getAs<FunctionProtoType>())
+AST_MATCHER(FunctionType, hasDynamicExceptionSpec) {
+  if (const auto *FnTy = dyn_cast<FunctionProtoType>(&Node))
     return FnTy->hasDynamicExceptionSpec();
   return false;
 }
 
 /// \brief Matches functions that have a non-throwing exception specification.
 ///
 /// Given:
 /// \code
 ///   void f();
 ///   void g() noexcept;
 ///   void h() throw();
 ///   void i() throw(int);
 ///   void j() noexcept(false);
 /// \endcode
 /// functionDecl(isNoThrow())
 ///   matches the declarations of g, and h, but not f, i or j.
 AST_MATCHER(FunctionDecl, isNoThrow) {
   const auto *FnTy = Node.getType()->getAs<FunctionProtoType>();
 
   // If the function does not have a prototype, then it is assumed to be a
   // throwing function (as it would if the function did not have any exception
   // specification).
   if (!FnTy)
     return false;
 
   // Assume the best for any unresolved exception specification.
   if (isUnresolvedExceptionSpec(FnTy->getExceptionSpecType()))
     return true;
 
   return FnTy->isNothrow(Node.getASTContext());
 }

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

TEST(Member, UnderstandsAccess) {
 
   // Non-member decls have the AccessSpecifier AS_none and thus aren't matched.
   EXPECT_TRUE(notMatches("int i;", varDecl(isPublic(), hasName("i"))));
   EXPECT_TRUE(notMatches("int i;", varDecl(isProtected(), hasName("i"))));
   EXPECT_TRUE(notMatches("int i;", varDecl(isPrivate(), hasName("i"))));
 }
 
 TEST(hasDynamicExceptionSpec, MatchesDynamicExceptionSpecifications) {
-  EXPECT_TRUE(notMatches("void f();", functionDecl(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(notMatches("void f();", functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(notMatches("void g() noexcept;",
-                         functionDecl(hasDynamicExceptionSpec())));
+                         functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(notMatches("void h() noexcept(true);",
-                         functionDecl(hasDynamicExceptionSpec())));
+                         functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(notMatches("void i() noexcept(false);",
-                         functionDecl(hasDynamicExceptionSpec())));
+                         functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
-      matches("void j() throw();", functionDecl(hasDynamicExceptionSpec())));
+      matches("void j() throw();", functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
-      matches("void k() throw(int);", functionDecl(hasDynamicExceptionSpec())));
+      matches("void k() throw(int);", functionType(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
-      matches("void l() throw(...);", functionDecl(hasDynamicExceptionSpec())));
+      matches("void l() throw(...);", functionType(hasDynamicExceptionSpec())));
 }
 
 TEST(HasObjectExpression, DoesNotMatchMember) {
   EXPECT_TRUE(notMatches(
     "class X {}; struct Z { X m; }; void f(Z z) { z.m; }",
     memberExpr(hasObjectExpression(hasType(recordDecl(hasName("X")))))));
 }