[PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 19 07:00:05 PST 2016


aaron.ballman added a comment.

In http://reviews.llvm.org/D8149#329791, @LegalizeAdulthood wrote:

> Is there any reason we can't proceed with the patch as-is and then see if it can be refactored into an overload of `hasType`?


The patch currently includes hasUnderlyingType() which we don't want because hasType() is the logically correct place to expose that functionality (it would be providing duplicate functionality with no benefit to expose hasUnderlyingType()). If you removed hasUnderlyingType() and split the hasType() changes into its own patch, we can easily proceed with the function prototype changes (AFAICT, there's only one outstanding test to add for that and it otherwise looks fine to me).


================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:1567
@@ -1566,1 +1566,3 @@
+  EXPECT_TRUE(matches("void f(...);", functionDecl(parameterCountIs(0))));
+  EXPECT_TRUE(matches("void f(int, ...);", functionDecl(parameterCountIs(1))));
 }
----------------
aaron.ballman wrote:
> What should be the outcome of this test (matches or not matches)?
> ```
> matchesC("void f();", functionDecl(parameterCountIs(0)));
> ```
> (Note, it's using C where this is variadic.) My guess is that it should match, but I'd like to make sure (and have a test for it).
I still don't see a `matchesC()` test case for `functionDecl(parameterCountIs()` on a varargs function, so I don't believe this comment is Done yet. (I do see one for getting to it through functionProtoType()).


http://reviews.llvm.org/D8149





More information about the cfe-commits mailing list