[cfe-dev] Matching std::set::iterator

Manuel Klimek klimek at google.com
Mon Oct 22 05:45:19 PDT 2012


On Wed, Oct 17, 2012 at 7:11 PM, Gábor Horváth <xazax.hun at gmail.com> wrote:

> To be correct, the code that I'm using for test, looks like this one:
>
>     std::set<int>::iterator it2 = s.begin();
>     find(it2,s.end(),i);


Looking at the AST:
    (CXXConstructExpr 0x489b058 <col:13> 'struct
std::_Rb_tree_const_iterator<int>':'struct
std::_Rb_tree_const_iterator<int>''void (const struct
std::_Rb_tree_const_iterator<int> &) throw()'
      (ImplicitCastExpr 0x489b040 <col:13> 'const struct
std::_Rb_tree_const_iterator<int>' lvalue <NoOp>
        (DeclRefExpr 0x489aa10 <col:13> 'std::set<int>::iterator':'struct
std::_Rb_tree_const_iterator<int>' lvalue Var 0x48974d0 'it'
'std::set<int>::iterator':'struct std::_Rb_tree_const_iterator<int>')))

I'd guess that after the ImplicitCastExpr the type is not the typedef'ed
type any more...

Cheers,
/Manuel


>
> On 17 October 2012 19:00, Gábor Horváth <xazax.hun at gmail.com> wrote:
>
>> Hi!
>>
>> I just tried out the new typedefType matcher. However there is one issue
>> I encountered, maybe  it is not intended to be used this way?
>>
>> This matcher matches:
>> callExpr(allOf(callee(functionDecl(hasName("std::find"))),
>>                                        hasArgument(0, hasType(type()))))
>>
>> However this matcher do not match:
>> callExpr(allOf(callee(functionDecl(hasName("std::find"))),
>>                                                           hasArgument(0,
>> hasType(typedefType()))))
>>
>> Do you have any idea what might be the problem? It compiles both ways.
>>
>> Thanks,
>> Gábor
>>
>>
>>
>> On 12 October 2012 11:59, Manuel Klimek <klimek at google.com> wrote:
>>
>>>
>>> On Fri, Oct 12, 2012 at 10:48 AM, Gábor Horváth <xazax.hun at gmail.com>wrote:
>>>
>>>> @Hal
>>>>
>>>> I'm using the tooling library, which is primarily for stand-alone
>>>> tools, however I think it is also possible to use the matchers in the
>>>> frontend, however I don't know, if it is advised (maybe Daniel or Manuel
>>>> can tell this). If it isn't advised, it will be better to use a recursive
>>>> AST visitor for this reason.
>>>>
>>>
>>> It's not yet adviced - we're working on that, but we want to have Doug &
>>> co. fully on board. One feature that's missing for that is being able to
>>> run matchers on arbitrary parts of the AST (not only the full AST).
>>>
>>>
>>>>
>>>> On 11 October 2012 16:06, Hal Finkel <hfinkel at anl.gov> wrote:
>>>>
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>> > From: "Gábor Horváth" <xazax.hun at gmail.com>
>>>>> > To: "Manuel Klimek" <klimek at google.com>
>>>>> > Cc: "Clang Developers" <cfe-dev at cs.uiuc.edu>
>>>>> > Sent: Thursday, October 11, 2012 7:39:50 AM
>>>>> > Subject: Re: [cfe-dev] Matching std::set::iterator
>>>>> >
>>>>> >
>>>>> > Yeah, I need this for a tool for my thesis (finding common stl
>>>>> > mistakes in code), but I can just use for example
>>>>> > "std::_Rb_tree_const_iterator", and mention in my thesis, the
>>>>> > current implementation only supports one certain STL implementation,
>>>>> > and will be improved in the future.
>>>>> >
>>>>> > But when some experimental patches are available, I will be happy to
>>>>> > test them out.
>>>>> >
>>>>>
>>>>> [quasi-off-topic] I'm interested in how this turns out, not only
>>>>> because I think that STL warnings will be a great thing to have, but also
>>>>> because I'm interested in this optimization: I'd like to output TBAA
>>>>> metadata so that the backend understands that pointers into distinct
>>>>> std::vector objects (and maybe other containers?) don't alias. It seems
>>>>> that implementing this will require the same kind of technology as what
>>>>> you're doing.
>>>>>
>>>>>  -Hal
>>>>>
>>>>> >
>>>>> >
>>>>> > On 11 October 2012 14:32, Manuel Klimek < klimek at google.com > wrote:
>>>>> >
>>>>> >
>>>>> >
>>>>> > On Thu, Oct 11, 2012 at 1:28 PM, Gábor Horváth < xazax.hun at gmail.com
>>>>> > > wrote:
>>>>> > > From my point of view, maybe something like matchesNameOrTypedef
>>>>> > > would be
>>>>> > > clean/easy. However I will be happy with any approach that provides
>>>>> > > an
>>>>> > > obvious interface. I think not being able to match typedefs is one
>>>>> > > of the
>>>>> > > biggest shortcomings of the matchers right now.
>>>>> >
>>>>> > Can you work around that by specifying the names it has been
>>>>> > typedef'ed to for now?
>>>>> >
>>>>> >
>>>>> >
>>>>> > >
>>>>> > >
>>>>> > > On 11 October 2012 14:14, Manuel Klimek < klimek at google.com >
>>>>> > > wrote:
>>>>> > >>
>>>>> > >> On Thu, Oct 11, 2012 at 12:03 PM, Daniel Jasper <
>>>>> > >> djasper at google.com >
>>>>> > >> wrote:
>>>>> > >>>
>>>>> > >>> We should make that decisions together with the Type/TypeLoc
>>>>> > >>> matching.
>>>>> > >>> I think looking through typedefs needs to be done with matchers
>>>>> > >>> for
>>>>> > >>> types. matchesName()/hasName() on declarations should always look
>>>>> > >>> at
>>>>> > >>> the name of the type used for the declaration and not at an alias
>>>>> > >>> defined in a typedef.
>>>>> > >>
>>>>> > >>
>>>>> > >>
>>>>> > >> I'm not sure I agree. It seems like the name property is really
>>>>> > >> one of the
>>>>> > >> decl or typedef, not one of the type.
>>>>> > >>
>>>>> > >> I'm curious about other opinions, though...
>>>>> > >>
>>>>> > >> Cheers,
>>>>> > >> /Manuel
>>>>> > >>
>>>>> > >>>
>>>>> > >>>
>>>>> > >>> Cheers,
>>>>> > >>> Daniel
>>>>> > >>>
>>>>> > >>> On Thu, Oct 11, 2012 at 11:47 AM, Manuel Klimek <
>>>>> > >>> klimek at google.com >
>>>>> > >>> wrote:
>>>>> > >>> > On Thu, Oct 11, 2012 at 11:05 AM, Gábor Horváth <
>>>>> > >>> > xazax.hun at gmail.com >
>>>>> > >>> > wrote:
>>>>> > >>> >> Hi!
>>>>> > >>> >>
>>>>> > >>> >> It looks like
>>>>> > >>> >> ...hasType(namedDecl(matchesName("std::set.*iterator")))
>>>>> > >>> >> ...
>>>>> > >>> >> only tries to match ::std::_Rb_tree_const_iterator, but not
>>>>> > >>> >> std::set<foobar>::iterator, so I basicaly can't match
>>>>> > >>> >> typedefs. I
>>>>> > >>> >> think it
>>>>> > >>> >> is not possible to match on typedef-ed types right now.
>>>>> > >>> >>
>>>>> > >>> >> There is one test case:
>>>>> > >>> >> EXPECT_TRUE(matches("typedef int X;", NamedX));
>>>>> > >>> >>
>>>>> > >>> >> However it matches a typedef declaration. However if we wan't
>>>>> > >>> >> to match
>>>>> > >>> >> something like "X i;" later on, we can not do that, however it
>>>>> > >>> >> would
>>>>> > >>> >> be
>>>>> > >>> >> extremely useful.
>>>>> > >>> >>
>>>>> > >>> >> Any comments on this?
>>>>> > >>> >
>>>>> > >>> > We need to write a matcher that supports this. We already have
>>>>> > >>> > some of
>>>>> > >>> > the supporting code for isDerivedFrom which looks through
>>>>> > >>> > typedefs.
>>>>> > >>> > We'll basically want to have a matcher matchesNameOrTypedef
>>>>> > >>> > (minus
>>>>> > >>> > finding a better name for the matcher ;) that does what you
>>>>> > >>> > want.
>>>>> > >>> >
>>>>> > >>> > Cheers,
>>>>> > >>> > /Manuel
>>>>> > >>> >
>>>>> > >>> >>
>>>>> > >>> >> Thanks,
>>>>> > >>> >> Gábor
>>>>> > >>> >>
>>>>> > >>> >>
>>>>> > >>> >>
>>>>> > >>> >> On 11 October 2012 09:39, Gábor Horváth < xazax.hun at gmail.com
>>>>> > >>> >> > wrote:
>>>>> > >>> >>>
>>>>> > >>> >>> Hi!
>>>>> > >>> >>>
>>>>> > >>> >>> More details on matching template specializations. After
>>>>> > >>> >>> further
>>>>> > >>> >>> trials I
>>>>> > >>> >>> even tried this piece of code:
>>>>> > >>> >>>
>>>>> > >>> >>>
>>>>> > >>> >>> MatcherProxy StlCOAPPred::getMatcher()
>>>>> > >>> >>> {
>>>>> > >>> >>> TypeMatcher type = unless(anything());
>>>>> > >>> >>>
>>>>> > >>> >>> for(const auto& e : gContainers)
>>>>> > >>> >>> type = anyOf(hasDeclaration(recordDecl(hasName(e))), type);
>>>>> > >>> >>>
>>>>> > >>> >>> return id("id",
>>>>> > >>> >>> varDecl(allOf(hasType(
>>>>> > >>> >>>
>>>>> > >>> >>> classTemplateSpecializationDecl(hasAnyTemplateArgument(
>>>>> > >>> >>>
>>>>> > >>> >>>
>>>>> > >>> >>>
>>>>> refersToType(hasDeclaration(hasDescendant(recordDecl(hasName("std::auto_ptr")))))))),
>>>>> > >>> >>> hasType(type))));
>>>>> > >>> >>> }
>>>>> > >>> >>>
>>>>> > >>> >>> If I remove the hasDescendant, it works flawlessly for
>>>>> > >>> >>> example for
>>>>> > >>> >>> vector<auto_ptr<int>>, however it will not fork for
>>>>> > >>> >>> vector<vector<auto_ptr<int>>>. If I add that hasDescendant it
>>>>> > >>> >>> will
>>>>> > >>> >>> not match
>>>>> > >>> >>> anything. The same applies to the code I pasted in my mail
>>>>> > >>> >>> earlier.
>>>>> > >>> >>> If you
>>>>> > >>> >>> have any idea what could cause this issue, please tell me.
>>>>> > >>> >>>
>>>>> > >>> >>> Thanks,
>>>>> > >>> >>> Gábor
>>>>> > >>> >>>
>>>>> > >>> >>>
>>>>> > >>> >>>
>>>>> > >>> >>> On 10 October 2012 15:37, Gábor Horváth <
>>>>> xazax.hun at gmail.com
>>>>> > >>> >>> > wrote:
>>>>> > >>> >>>>
>>>>> > >>> >>>> For the second one, I altered my snippet:
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> > >>> >>>> MatcherProxy StlCOAPPred::getMatcher()
>>>>> > >>> >>>> {
>>>>> > >>> >>>> TypeMatcher type = unless(anything());
>>>>> > >>> >>>>
>>>>> > >>> >>>> for(const auto& e : gContainers)
>>>>> > >>> >>>> type = anyOf(hasDeclaration(recordDecl(hasName(e))), type);
>>>>> > >>> >>>>
>>>>> > >>> >>>> return id("id",
>>>>> > >>> >>>> varDecl(allOf(hasType(recordDecl(hasDescendant(
>>>>> > >>> >>>>
>>>>> > >>> >>>> classTemplateSpecializationDecl(hasAnyTemplateArgument(
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> refersToType(hasDeclaration(recordDecl(hasName("std::auto_ptr"))))))))),
>>>>> > >>> >>>> hasType(type))));
>>>>> > >>> >>>> }
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> > >>> >>>> But it does not seems to work. It do not give me any
>>>>> > >>> >>>> matches.
>>>>> > >>> >>>>
>>>>> > >>> >>>> For the first one, I futher will investigate it later today,
>>>>> > >>> >>>> however
>>>>> > >>> >>>> my
>>>>> > >>> >>>> bet would be that, it tries to match
>>>>> > >>> >>>> "std::_Rb_tree_const_iterator".
>>>>> > >>> >>>>
>>>>> > >>> >>>> Thanks,
>>>>> > >>> >>>> Gábor
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> > >>> >>>> On 10 October 2012 13:55, Daniel Jasper <
>>>>> djasper at google.com
>>>>> > >>> >>>> > wrote:
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> +cfe-dev, please remember to include
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> I don't see anything generally wrong with your approach to
>>>>> > >>> >>>>> match
>>>>> > >>> >>>>> iterators. What I would do to debug this is locally editing
>>>>> > >>> >>>>> ASTMatchers.h to add a "llvm::outs() << FullName;" debug
>>>>> > >>> >>>>> output
>>>>> > >>> >>>>> into
>>>>> > >>> >>>>> the matchesName matcher. That way, you can see what it is
>>>>> > >>> >>>>> actually
>>>>> > >>> >>>>> trying to match.
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> The second question should work like:
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> varDecl(hasType(recordDecl(
>>>>> > >>> >>>>>
>>>>> > >>> >>>>>
>>>>> hasDescendent(classTemplateSpecialization(hasAnyTemplateArgument(
>>>>> > >>> >>>>> refersToType(hasName("std::auto_ptr")))))))).bind("id")
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> This is just as a general idea, it might not yet be
>>>>> > >>> >>>>> correct.
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> Cheers,
>>>>> > >>> >>>>> Daniel
>>>>> > >>> >>>>>
>>>>> > >>> >>>>> On Wed, Oct 10, 2012 at 1:23 PM, Gábor Horváth
>>>>> > >>> >>>>> < xazax.hun at gmail.com >
>>>>> > >>> >>>>> wrote:
>>>>> > >>> >>>>> > Hi!
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > I want to create a matcher to match things like
>>>>> > >>> >>>>> > std::set<int>::iterator.
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > I come up with this one: ...
>>>>> > >>> >>>>> > hasType(namedDecl(matchesName("std::set.*iterator"))) ...
>>>>> > >>> >>>>> > however it did not give me any match.
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > Using the internal name of the iterator class like
>>>>> > >>> >>>>> > std::_Rb_tree_const_iterator would work, however that is
>>>>> > >>> >>>>> > implementation
>>>>> > >>> >>>>> > defined, so I do not want to rely on that one.
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > Do you have any idea what am I doing wrong?
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > My another question is, for example if I want to search
>>>>> > >>> >>>>> > for
>>>>> > >>> >>>>> > auto_ptr
>>>>> > >>> >>>>> > as
>>>>> > >>> >>>>> > template arguments in containers, I want to match
>>>>> > >>> >>>>> > vector<auto_ptr<int>> and
>>>>> > >>> >>>>> > also vector<vector<auto_ptr<int>>> ... etc.
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > Is there any proper way to do it? My current solution:
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > MatcherProxy StlCOAPPred::getMatcher()
>>>>> > >>> >>>>> > {
>>>>> > >>> >>>>> > TypeMatcher type = unless(anything());
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > for(const auto& e : gContainers)
>>>>> > >>> >>>>> > type = anyOf(hasDeclaration(recordDecl(hasName(e))),
>>>>> > >>> >>>>> > type);
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > auto templateSpecWithArgument = [](DeclarationMatcher
>>>>> > >>> >>>>> > decl) ->
>>>>> > >>> >>>>> > DeclarationMatcher {
>>>>> > >>> >>>>> > return
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> classTemplateSpecializationDecl(hasAnyTemplateArgument(refersToType(hasDeclaration(decl))));
>>>>> > >>> >>>>> > };
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > // 1, 2, 3 times embedded
>>>>> > >>> >>>>> > DeclarationMatcher decl =
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> anyOf(templateSpecWithArgument(recordDecl(hasName("std::auto_ptr"))),
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> templateSpecWithArgument(templateSpecWithArgument(recordDecl(hasName("std::auto_ptr")))),
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> templateSpecWithArgument(templateSpecWithArgument(templateSpecWithArgument(recordDecl(hasName("std::auto_ptr"))))));
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > return
>>>>> > >>> >>>>> > id("id",varDecl(allOf(hasType(decl),hasType(type))));
>>>>> > >>> >>>>> > }
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > I use templateSpecWithArgument, and nesting it. I could
>>>>> > >>> >>>>> > write a
>>>>> > >>> >>>>> > loop
>>>>> > >>> >>>>> > to do
>>>>> > >>> >>>>> > this nesting several times, however I think that could
>>>>> > >>> >>>>> > degrade
>>>>> > >>> >>>>> > the
>>>>> > >>> >>>>> > performance significantly (I did not measure yet).
>>>>> > >>> >>>>> >
>>>>> > >>> >>>>> > Thanks
>>>>> > >>> >>>>> > Gábor
>>>>> > >>> >>>>
>>>>> > >>> >>>>
>>>>> > >>> >>>
>>>>> > >>> >>
>>>>> > >>> >>
>>>>> > >>> >> _______________________________________________
>>>>> > >>> >> cfe-dev mailing list
>>>>> > >>> >> cfe-dev at cs.uiuc.edu
>>>>> > >>> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>>> > >>> >>
>>>>> > >>
>>>>> > >>
>>>>> > >
>>>>> >
>>>>> >
>>>>> > _______________________________________________
>>>>> > cfe-dev mailing list
>>>>> > cfe-dev at cs.uiuc.edu
>>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>>> >
>>>>>
>>>>> --
>>>>> Hal Finkel
>>>>> Postdoctoral Appointee
>>>>> Leadership Computing Facility
>>>>> Argonne National Laboratory
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121022/b2ccc643/attachment.html>


More information about the cfe-dev mailing list