[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

3254

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

 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.
+functionDecl(hasDynamicExceptionSpec()) and
+  functionProtoType(hasDynamicExceptionSpec())
+  match 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;".

 <tr><td colspan="4" class="doc" id="isNoThrow0"><pre>Matches functions that have a non-throwing exception specification.
 
 Given:
   void f();
   void g() noexcept;
   void h() throw();
   void i() throw(int);
   void j() noexcept(false);
-functionDecl(isNoThrow())
-  matches the declarations of g, and h, but not f, i or j.
+functionDecl(isNoThrow()) and functionProtoType(isNoThrow())
+  match the declarations of g, and h, but not f, i or j.
 </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('isTemplateInstantiation0')"><a name="isTemplateInstantiation0Anchor">isTemplateInstantiation</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="isTemplateInstantiation0"><pre>Matches template instantiations of function, class, or static
 member variable template instantiations.
 
 Given
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_1FunctionProtoType.html">FunctionProtoType</a>&gt;</td><td class="name" onclick="toggle('hasDynamicExceptionSpec1')"><a name="hasDynamicExceptionSpec1Anchor">hasDynamicExceptionSpec</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="hasDynamicExceptionSpec1"><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()) and
+  functionProtoType(hasDynamicExceptionSpec())
+  match 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_1FunctionProtoType.html">FunctionProtoType</a>&gt;</td><td class="name" onclick="toggle('isNoThrow1')"><a name="isNoThrow1Anchor">isNoThrow</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isNoThrow1"><pre>Matches functions that have a non-throwing exception specification.
+
+Given:
+  void f();
+  void g() noexcept;
+  void h() throw();
+  void i() throw(int);
+  void j() noexcept(false);
+functionDecl(isNoThrow()) and functionProtoType(isNoThrow())
+  match the declarations of g, and h, but not f, i or j.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionProtoType.html">FunctionProtoType</a>&gt;</td><td class="name" onclick="toggle('parameterCountIs1')"><a name="parameterCountIs1Anchor">parameterCountIs</a></td><td>unsigned N</td></tr>
 <tr><td colspan="4" class="doc" id="parameterCountIs1"><pre>Matches FunctionDecls and FunctionProtoTypes that have a
 specific parameter count.
 
 Given
   void f(int i) {}
   void g(int i, int j) {}
   void h(int i, int j);

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())
-///   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>())
+/// functionDecl(hasDynamicExceptionSpec()) and
+///   functionProtoType(hasDynamicExceptionSpec())
+///   match the declarations of j, k, and l, but not f, g, h, or i.
+AST_POLYMORPHIC_MATCHER(hasDynamicExceptionSpec,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                        FunctionProtoType)) {
+  if (const FunctionProtoType *FnTy = internal::getFunctionProtoType(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>();
+/// functionDecl(isNoThrow()) and functionProtoType(isNoThrow())
+///   match the declarations of g, and h, but not f, i or j.
+AST_POLYMORPHIC_MATCHER(isNoThrow,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                        FunctionProtoType)) {
+  const FunctionProtoType *FnTy = internal::getFunctionProtoType(Node);
 
   // 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());
+  return FnTy->isNothrow(Finder->getASTContext());
 }
 
 /// \brief Matches constexpr variable and function declarations.
 ///
 /// Given:
 /// \code
 ///   constexpr int foo = 42;
 ///   constexpr int bar();

include/clang/ASTMatchers/ASTMatchersInternal.h

 inline QualType getUnderlyingType(const ValueDecl &Node) {
   return Node.getType();
 }
 
 inline QualType getUnderlyingType(const TypedefNameDecl &Node) {
   return Node.getUnderlyingType();
 }
 
+/// \brief Unifies obtaining the FunctionProtoType pointer from both
+/// FunctionProtoType and FunctionDecl nodes..
+inline const FunctionProtoType *
+getFunctionProtoType(const FunctionProtoType &Node) {
+  return &Node;
+}
+
+inline const FunctionProtoType *getFunctionProtoType(const FunctionDecl &Node) {
+  return Node.getType()->getAs<FunctionProtoType>();
+}
+
 /// \brief Internal version of BoundNodes. Holds all the bound nodes.
 class BoundNodesMap {
 public:
   /// \brief Adds \c Node to the map with key \c ID.
   ///
   /// The node's base type should be in NodeBaseType or it will be unaccessible.
   void addNode(StringRef ID, const ast_type_traits::DynTypedNode& DynNode) {
     NodeMap[ID] = DynNode;

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

 
 TEST(IsNoThrow, MatchesNoThrowFunctionDeclarations) {
   EXPECT_TRUE(notMatches("void f();", functionDecl(isNoThrow())));
   EXPECT_TRUE(notMatches("void f() throw(int);", functionDecl(isNoThrow())));
   EXPECT_TRUE(
     notMatches("void f() noexcept(false);", functionDecl(isNoThrow())));
   EXPECT_TRUE(matches("void f() throw();", functionDecl(isNoThrow())));
   EXPECT_TRUE(matches("void f() noexcept;", functionDecl(isNoThrow())));
+
+  EXPECT_TRUE(notMatches("void f();", functionProtoType(isNoThrow())));
+  EXPECT_TRUE(notMatches("void f() throw(int);", functionProtoType(isNoThrow())));
+  EXPECT_TRUE(
+    notMatches("void f() noexcept(false);", functionProtoType(isNoThrow())));
+  EXPECT_TRUE(matches("void f() throw();", functionProtoType(isNoThrow())));
+  EXPECT_TRUE(matches("void f() noexcept;", functionProtoType(isNoThrow())));
 }
 
 TEST(isConstexpr, MatchesConstexprDeclarations) {
   EXPECT_TRUE(matches("constexpr int foo = 42;",
                       varDecl(hasName("foo"), isConstexpr())));
   EXPECT_TRUE(matches("constexpr int bar();",
                       functionDecl(hasName("bar"), isConstexpr())));
 }
TEST(hasDynamicExceptionSpec, MatchesDynamicExceptionSpecifications) {
   EXPECT_TRUE(notMatches("void i() noexcept(false);",
                          functionDecl(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
       matches("void j() throw();", functionDecl(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
       matches("void k() throw(int);", functionDecl(hasDynamicExceptionSpec())));
   EXPECT_TRUE(
       matches("void l() throw(...);", functionDecl(hasDynamicExceptionSpec())));
+
+  EXPECT_TRUE(notMatches("void f();", functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(notMatches("void g() noexcept;",
+                         functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(notMatches("void h() noexcept(true);",
+                         functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(notMatches("void i() noexcept(false);",
+                         functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(
+      matches("void j() throw();", functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(
+      matches("void k() throw(int);", functionProtoType(hasDynamicExceptionSpec())));
+  EXPECT_TRUE(
+      matches("void l() throw(...);", functionProtoType(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")))))));
 }