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

Richard via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 4 09:31:48 PST 2016


LegalizeAdulthood added inline comments.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4990
@@ +4989,3 @@
+  EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;",
+                      typedefDecl(hasUnderlyingType(asString("int")))));
+  EXPECT_TRUE(
----------------
aaron.ballman wrote:
> 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.)
I'm happy to make the improvement, but I don't know how.  I simply call the `Node.underlyingType()`, just like `hasType` calls `Node.getType()`.  I don't know why they are different.


http://reviews.llvm.org/D8149





More information about the cfe-commits mailing list