sbenza added inline comments.
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4994
@@ +4993,3 @@
+ EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;",
+ typedefDecl(hasUnderlyingType(asString("int")))));
+ EXPECT_TRUE(matches("typedef const int T;",
----------------
LegalizeAdulthood wrote:
> sbenza wrote:
> > aaron.ballman wrote:
> > > LegalizeAdulthood wrote:
> > > > aaron.ballman wrote:
> > > > > LegalizeAdulthood wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Thank you for those examples! Given the following code:
> > > > > > > ```
> > > > > > > typedef int foo;
> > > > > > > typedef foo bar;
> > > > > > >
> > > > > > > bar i;
> > > > > > > ```
> > > > > > > clang-query> match varDecl(hasType(asString("int")))
> > > > > > > 0 matches.
> > > > > > > clang-query> match varDecl(hasType(asString("foo")))
> > > > > > > 0 matches.
> > > > > > > clang-query> match varDecl(hasType(asString("bar")))
> > > > > > >
> > > > > > > Match #1:
> > > > > > >
> > > > > > > E:\Desktop\t.cpp:4:1: note: "root" binds here
> > > > > > > bar i;
> > > > > > > ^~~~~
> > > > > > > 1 match.
> > > > > > >
> > > > > > > So hasType() looks at what the immediate type is for the
> > > > > > > declaration (which we document, yay us!). Based on that, I don't
> > > > > > > think hasUnderlyingType() makes sense -- you should modify
> > > > > > > hasType() to work on a TypedefNameDecl (not just a TypedefDecl!)
> > > > > > > so that it looks at the immediate type of the type definition. I
> > > > > > > would expect your tests then to result in:
> > > > > > > ```
> > > > > > > 1: typedef void (fn)(void);
> > > > > > > 2: typedef fn foo;
> > > > > > > 3: typedef int bar;
> > > > > > > 4: typedef int (f);
> > > > > > > 5: typedef int (fn2)(int);
> > > > > > > clang-query> match typedefDecl(hasType(asString("int")))
> > > > > > >
> > > > > > > Match #1:
> > > > > > >
> > > > > > > /tmp/a.cpp:3:1: note: "root" binds here
> > > > > > > typedef int bar;
> > > > > > > ^~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > Match #2:
> > > > > > >
> > > > > > > /tmp/a.cpp:4:1: note: "root" binds here
> > > > > > > typedef int (f);
> > > > > > > ^~~~~~~~~~~~~~~
> > > > > > > 2 matches.
> > > > > > > clang-query> match typedefDecl(hasType(typedefType()))
> > > > > > >
> > > > > > > Match #1:
> > > > > > >
> > > > > > > /tmp/a.cpp:2:1: note: "root" binds here
> > > > > > > typedef fn foo;
> > > > > > > ^~~~~~~~~~~~~~
> > > > > > > 1 match.
> > > > > > > clang-query> match typedefDecl(hasType(parenType()))
> > > > > > >
> > > > > > > Match #1:
> > > > > > >
> > > > > > > /tmp/a.cpp:1:1: note: "root" binds here
> > > > > > > typedef void (fn)(void);
> > > > > > > ^~~~~~~~~~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > Match #2:
> > > > > > >
> > > > > > > /tmp/a.cpp:4:1: note: "root" binds here
> > > > > > > typedef int (f);
> > > > > > > ^~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > Match #3:
> > > > > > >
> > > > > > > /tmp/a.cpp:5:1: note: "root" binds here
> > > > > > > typedef int (fn2)(int);
> > > > > > > ^~~~~~~~~~~~~~~~~~~~~~
> > > > > > > 3 matches.
> > > > > > > clang-query> match
> > > > > > > typedefDecl(hasType(parenType(innerType(functionType()))))
> > > > > > >
> > > > > > > Match #1:
> > > > > > >
> > > > > > > /tmp/a.cpp:1:1: note: "root" binds here
> > > > > > > typedef void (fn)(void);
> > > > > > > ^~~~~~~~~~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > Match #2:
> > > > > > >
> > > > > > > /tmp/a.cpp:5:1: note: "root" binds here
> > > > > > > typedef int (fn2)(int);
> > > > > > > ^~~~~~~~~~~~~~~~~~~~~~
> > > > > > > 2 matches.
> > > > > > > ```
> > > > > > > The end results are the same, so this is just changing the way
> > > > > > > the information is surfaced to the user that is logically
> > > > > > > consistent. ValueDecls have an immediate type, and so do
> > > > > > > TypedefDecls. By using TypedefNameDecl instead of TypedefDecl,
> > > > > > > this also covers the case where hasType() is useful for an
> > > > > > > alias-declaration. (We don't expose the matcher for that yet, but
> > > > > > > it seems quite reasonable to add in the future, and it would be
> > > > > > > nice for hasType to automatically work with that.)
> > > > > > >
> > > > > > > You can implement this with a helper function to handle
> > > > > > > abstracting away the call to getType() vs getUnderlyingType(),
> > > > > > > then updating the hasType() matchers to use it. Something like:
> > > > > > > ```
> > > > > > > template <typename Ty>
> > > > > > > struct UnderlyingTypeGetter {
> > > > > > > static QualType get(const Ty &Node) {
> > > > > > > return Node.getType();
> > > > > > > }
> > > > > > > };
> > > > > > >
> > > > > > > template <>
> > > > > > > QualType UnderlyingTypeGetter<TypedefNameDecl>::get(const
> > > > > > > TypedefNameDecl &Node) {
> > > > > > > return Node.getUnderlyingType();
> > > > > > > }
> > > > > > > ```
> > > > > > > (Somewhere in ASTMatchersInternal.h most likely.)
> > > > > > >
> > > > > > When I try to extend `hasType` to work on `TypedefDecl`, I get this
> > > > > > error:
> > > > > >
> > > > > > ```
> > > > > > error: static assertion failed: right polymorphic conversion
> > > > > > static_assert(TypeListContainsSuperOf<ReturnTypes, T>::value,
> > > > > > ```
> > > > > >
> > > > > > ...because `TypedefDecl` is derived from `NamedDecl` and the
> > > > > > existing definition for `hasType` looks like this:
> > > > > >
> > > > > > ```
> > > > > > AST_POLYMORPHIC_MATCHER_P_OVERLOAD(hasType,
> > > > > >
> > > > > > AST_POLYMORPHIC_SUPPORTED_TYPES(Expr,
> > > > > >
> > > > > > ValueDecl),
> > > > > > internal::Matcher<Decl>,
> > > > > > InnerMatcher, 1) {
> > > > > > return qualType(hasDeclaration(InnerMatcher))
> > > > > > .matches(Node.getType(), Finder, Builder);
> > > > > > }
> > > > > > ```
> > > > > >
> > > > > > So I'll need some guidance on how to extend `hasType` to work for
> > > > > > `TypedefNamedDecl` nodes. I don't understand exactly what all
> > > > > > these nasty macros do. So far, I've simply made changes by
> > > > > > imitation, but my approach didn't work this time.
> > > > > This ({F1302460}) does all of what you need (sans documentation,
> > > > > testing, etc).
> > > > >
> > > > > (File should be attached, but if you need me to send it to you via
> > > > > email, I can do so -- I've never tried this with Phab before.)
> > > > What you had was very similar to what I attempted. You wrote:
> > > >
> > > > ```AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, TypdefNameDecl, ValueDecl)```
> > > >
> > > > and I wrote
> > > >
> > > > ```AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, ValueDecl, TypedefNameDecl)```
> > > >
> > > > so apparently the derivation relations between the arguments to this
> > > > macro are order dependent?
> > > I was being OCD and alphabetizing, but I think you may be right that
> > > order matters. @sbenza, may know more (if it's purposeful, we should
> > > document it).
> > Order should not matter.
> > We just iterate over the list to see if there is a matching type.
> > I could not reproduce the compiler error.
> The compile error occurs in the tests, not in the implementation. See the
> attached shell output. What am I doing wrong? I don't know what to do about
> the error.
>
> {F1318634}
There are two overloads of hasType(). You only fixed one of them in that patch
and the test happens to use the other one.
http://reviews.llvm.org/D8149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
