LegalizeAdulthood added inline comments.
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4283
@@ +4282,3 @@
+ EXPECT_TRUE(matches("void f(int i);", functionProtoType()));
+ EXPECT_TRUE(matches("void f();", functionProtoType(parameterCountIs(0))));
+ EXPECT_TRUE(notMatchesC("void f();", functionProtoType()));
----------------
aaron.ballman wrote:
> > I don't understand what would be tested by adding a test for
> > parameterCountIs() on a function that doesn't have a prototype since it is
> > a nested matcher passed to functionProtoType() and since the function
> > doesn't have a prototype, that outer matcher won't match.
>
> Not according to the AST matcher that was implemented -- it supports
> FunctionProtoType and FunctionDecl. I want to make sure that if you get to
> parameterCountIs() through functionDecl() that it doesn't fail.
Ah, OK, you were asking for a test on `functionDecl`, not `functionProtoType`.
I can do that. I guess what threw me off is that the comment was attached to
the prototype tests.
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4992
@@ +4991,3 @@
+ EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;",
+ typedefDecl(hasUnderlyingType(asString("int")))));
+ EXPECT_TRUE(matches("typedef const int T;",
----------------
aaron.ballman wrote:
> > 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.
>
> hasType() and hasUnderlyingType() have different operational semantic
> implications, at least to me. hasType() asks, "ultimately, what is the type
> of this thing?", and Node.underlyingType() answers that. To me,
> hasUnderlyingType() asks, "in the chain of types that this typedef refers to,
> does this thing match any of them?", and Node.underlyingType() does not
> answer that -- it only looks at the final desugared type. The difference we
> want is that hasType() continues to look at the final type, but
> hasUnderlyingType() needs to do more work to look at intermediary types and
> terminate that loop when the answer is "yes, it has this type" or "no, we
> can't desugar any further."
>
> If we don't care about the intermediate types for your needs, then I don't
> see the point to hasUnderlyingType() being an AST matcher at all. hasType()
> does exactly what is needed, and you can instead modify that to accept a
> TypedefDecl in addition to Expr and ValueDecl. However, I still see value in
> hasUnderlyingType() because typedef chains can be long and complex (like in
> the Win32 APIs), and being able to query for intermediary types would be
> useful. e.g., I want to know about all types that have an intermediary type
> of DWORD (which itself is a typedef for an unsigned integer type). hasType()
> will always go to the final type, making such a matcher impossible without
> something like hasUnderlyingType().
I just named the matcher after the
[accessor](http://clang.llvm.org/doxygen/classclang_1_1TypedefNameDecl.html#a5fccedff6d3854db365a540145029158)
on a typedefDecl node. To me it's just a narrowing matcher on that node that
gives you access to whatever the accessor returns.
Things may have changed in the meantime, but when I originally created this
patch you couldn't do `typedefDecl(hasType(asString("int")))`. It doesn't
compile. Since `hasType` just matched against the value of whatever
`Node.getType()` returned it seemed natural to add a matcher called
`hasUnderlyingType` that just matched against the value of whatever
`Node.getUnderlyingType()` returned.
As I said, I'm happy to make the suggested improvement, but beyond simple
immitation of existing matchers, I have no idea how to traverse the encodings
of types within the guts of clang. This is what I mean when I say "I don't
know how". I understand conceptually what you are saying but I have no idea
how to express that in clang's code base.
http://reviews.llvm.org/D8149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
