aaron.ballman added inline comments.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2908
@@ -2893,1 +2907,3 @@
+/// \brief Matches \c FunctionDecls and FunctionProtoTypes that have a specific
+/// parameter count.
///
----------------
Can you document the expected parameter count for:
```
void f(int i, ...);
```
(I would expect it to report 1.)
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:4022
@@ +4021,3 @@
+/// functionProtoType()
+/// matches "int (*f)(int)" and the type of "g".
+AST_TYPE_MATCHER(FunctionProtoType, functionProtoType);
----------------
That's what I was expecting; can you update the comment to match reality?
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4282
@@ +4281,3 @@
+ EXPECT_TRUE(matches("void f(int i);", functionProtoType()));
+ EXPECT_TRUE(matches("void f();", functionProtoType(parameterCountIs(0))));
+ EXPECT_TRUE(
----------------
This isn't quite what I had in mind. The test you added is a good one to add,
but I wanted a test that functionProtoType() does *not* match f() (because it
has no prototype), and another test for parameterCountIs() that it does't
explode on a function without a prototype in C.
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4990
@@ +4989,3 @@
+ EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;",
+ typedefDecl(hasUnderlyingType(asString("int")))));
+ EXPECT_TRUE(
----------------
I would expect hasUnderlyingType to look through all of the type sugar, not
just the top layer of it (since existing matchers can implement the current
behavior by using hasType, I believe). I think the correct approach is to get
the underlying type, then loop to see whether that type matches, and if not,
strip off a layer of sugar and try again. Terminate the loop when there's no
more sugar to strip. The following should all match:
```
EXPECT_TRUE(
matches("typedef int foo; typedef foo bar; typedef bar baz;",
typedefDecl(hasUnderlyingType(asString("int")), hasName("bar"))));
EXPECT_TRUE(
matches("typedef int foo; typedef foo bar; typedef bar baz;",
typedefDecl(hasUnderlyingType(asString("foo")), hasName("baz"))));
EXPECT_TRUE(
matches("typedef int foo; typedef foo bar; typedef bar baz;",
typedefDecl(hasUnderlyingType(asString("int")), hasName("baz"))));
```
The other question I have is about qualifiers. Should this match?
```
EXPECT_TRUE(
matches("typedef const int foo;",
typedefDecl(hasUnderlyingType(asString("int")), hasName("foo"))));
```
Or should it require `asString("const int")`?
(Regardless of the answer, we should document the behavior and add a test with
qualifiers.)
http://reviews.llvm.org/D8149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
