I've commit in r247885 and r247886. I will add something to the
release notes, and watch the bots to see if any tests got missed
(since I did my development on Windows).
Thank you!
~Aaron
On Wed, Sep 16, 2015 at 7:49 PM, Manuel Klimek wrote:
> LG, ship it.
>
> On Wed, Sep 16, 2015 at 2:03 PM Aa
LG, ship it.
On Wed, Sep 16, 2015 at 2:03 PM Aaron Ballman
wrote:
> Attached is an updated patch for clang-tools-extra that does not have
> my in-progress, not-related-to-any-of-this code in it. ;-)
>
> ~Aaron
>
> On Wed, Sep 16, 2015 at 4:04 PM, Aaron Ballman
> wrote:
> > Quick ping. I know th
Attached is an updated patch for clang-tools-extra that does not have
my in-progress, not-related-to-any-of-this code in it. ;-)
~Aaron
On Wed, Sep 16, 2015 at 4:04 PM, Aaron Ballman wrote:
> Quick ping. I know this is a fairly gigantic patch, but I'm hoping for
> a relatively quick review turna
Quick ping. I know this is a fairly gigantic patch, but I'm hoping for
a relatively quick review turnaround because of potential merge
conflicts with people doing a fair amount of work on clang-tidy
lately. Everything should be pretty straight-forward (it's all just
renames, no semantic changes int
On Mon, Sep 14, 2015 at 2:41 PM, Manuel Klimek wrote:
>
> On Mon, Sep 14, 2015 at 2:29 PM Aaron Ballman
> wrote:
>
>> On Mon, Sep 14, 2015 at 4:38 PM, Manuel Klimek wrote:
>> >
>> >
>> > On Mon, Sep 14, 2015 at 12:26 PM Aaron Ballman
>> > wrote:
>> >>
>> >> On Mon, Sep 14, 2015 at 2:45 PM, Dani
On Mon, Sep 14, 2015 at 5:47 PM, Daniel Jasper wrote:
> Btw, I think generating them, potentially into several different headers to
> work around the compile time issue isn't such a bad idea.
I'm not going to start with this approach, but think it may be worth
exploring at some point. ;-)
~Aaron
On Mon, Sep 14, 2015 at 5:44 PM, Daniel Jasper wrote:
> Ok. I am happy with this then.
>
> (Just personally grumpy having to write
> cxxRecordDecl(has(cxxConstructorDecl(..))) in the future ;-) ).
I share your grumpiness about the cxxConstructorDecl, but probably
won't share it when we add objcCo
Btw, I think generating them, potentially into several different headers to
work around the compile time issue isn't such a bad idea.
On Mon, Sep 14, 2015 at 11:45 PM, Manuel Klimek wrote:
> Feel free to rename the AST nodes :)
>
> On Mon, Sep 14, 2015, 2:44 PM Daniel Jasper wrote:
>
>> Ok. I a
Feel free to rename the AST nodes :)
On Mon, Sep 14, 2015, 2:44 PM Daniel Jasper wrote:
> Ok. I am happy with this then.
>
> (Just personally grumpy having to write
> cxxRecordDecl(has(cxxConstructorDecl(..))) in the future ;-) ).
>
> On Mon, Sep 14, 2015 at 11:41 PM, Manuel Klimek wrote:
>
>>
Ok. I am happy with this then.
(Just personally grumpy having to write
cxxRecordDecl(has(cxxConstructorDecl(..))) in the future ;-) ).
On Mon, Sep 14, 2015 at 11:41 PM, Manuel Klimek wrote:
>
>
> On Mon, Sep 14, 2015 at 2:29 PM Aaron Ballman
> wrote:
>
>> On Mon, Sep 14, 2015 at 4:38 PM, Manue
On Mon, Sep 14, 2015 at 2:29 PM Aaron Ballman
wrote:
> On Mon, Sep 14, 2015 at 4:38 PM, Manuel Klimek wrote:
> >
> >
> > On Mon, Sep 14, 2015 at 12:26 PM Aaron Ballman
> > wrote:
> >>
> >> On Mon, Sep 14, 2015 at 2:45 PM, Daniel Jasper
> wrote:
> >> > By this point, I see that change might be
On Mon, Sep 14, 2015 at 4:38 PM, Manuel Klimek wrote:
>
>
> On Mon, Sep 14, 2015 at 12:26 PM Aaron Ballman
> wrote:
>>
>> On Mon, Sep 14, 2015 at 2:45 PM, Daniel Jasper wrote:
>> > By this point, I see that change might be profitable overall. However,
>> > lets
>> > completely map this out. Chan
; >> >>>>> >> >> better solution?
> >> >>>>> >> >>
> >> >>>>> >> >> Basically, I see a few ways to solve this (and there may be
> >> >>>>> >> >> other
> >> >>>>> >> >
gt; >> cxxRecordDecl. This does not match the AST because we have no
>> >>>>> >> >> StructDecl or UnionDecl types. The more I think about this
>> >>>>> >> >> option, the
>> >>>>> >> >> less I like it. It's easy to
t;> >>>>> >> >> to
>> >>>>> >> >> the AST.
>> >>>>> >> >> 2) Make recordDecl match RecordDecl, don't touch other
>> matchers.
>> >>>>> >> >> Add
>> >>>>> >>
>> >> >> break
> >>>>> >> >> code. I think that I prefer this approach, but it depends
> heavily
> >>>>> >> >> on
> >>>>> >> >> what "may break code" looks like in practice.
> >>>>> >> &
This doesn't match the AST because we will have
>>>>> >> >> matchers taking a RecordDecl when the AST would require a
>>>>> >> >> CXXRecordDecl, but is likely to break less code.
>>>>> >> >
&g
gt;> >>
>>>> >> If this is an acceptable failure rate, I can also update the failing
>>>> >> matchers to use cxxRecordDecl instead of recordDecl where applicable.
>>>> >> Doing some spot-checking of the failing code, the failures are ones
>>>> w
niel for another opinion. I think this is fine, but I'd prefer not
>>> to
>>> > end up in a meme :)
>>>
>>> FWIW, the vast majority of the errors are in ASTMatchersTests. There
>>> were only a few in actual real-world uses (71 in tests, 14 in re
>>
>> >> >> > Richard: if CXXRecordDecl was really an implementation detail, it
>> >> >> > would
>> >> >> > be
>> >> >> > hidden behind a RecordDecl class, as an implementation detail. The
>> >> >> >
n't matter - in the end, it's
> in
> >> >> > the
> >> >> > AST API.
> >> >> >
> >> >> >>
> >> >> >>
> >> >> >> ~Aaron
> >> >> >>
> >> >
;s in
>>> >> > the
>>> >> > AST API.
>>> >> >
>>> >> >>
>>> >> >>
>>> >> >> ~Aaron
>>> >> >>
>>> >> >> >
>>> >> >> > On Fri, Sep 11, 2015
gt; > wrote:
>> >> >> >>
>> >> >> >> Richard! We need an informed opinion :D
>> >> >> >>
>> >> >> >> On Fri, Sep 11, 2015 at 3:07 PM Aaron Ballman
>> >> >> >>
>> >> >> &
; >>> Ping?
> >> >> >>>
> >> >> >>> On Tue, Sep 8, 2015 at 9:26 AM, Manuel Klimek >
> >> >> >>> wrote:
> >> >> >>> > On Tue, Sep 8, 2015 at 3:23 PM Aaron Ballman
> >> >> >>> >
> >> >
gt;>
>> >> >>> >> On Tue, Sep 8, 2015 at 9:18 AM, Manuel Klimek
>> >> >>> >>
>> >> >>> >> wrote:
>> >> >>> >> > On Tue, Sep 8, 2015 at 2:23 PM Aaron Ballman
>> >> >>> >> >
>>
;>
> >> >>> >> >> On Tue, Sep 8, 2015 at 5:40 AM, Manuel Klimek
> >> >>> >> >>
> >> >>> >> >> wrote:
> >> >>> >> >> > Yea, we had that discussion a few times, an
>> > ended up in the state we're in.
>> >>> >> >> > We definitely had a time where we switched to just using the
>> >>> >> >> > exact
>> >>> >> >> > same
>> >>> >> >> > name
>> >&
>> > exact
> >>> >> >> > same
> >>> >> >> > name
> >>> >> >> > as the node's class name for the matchers.
> >>> >> >> > I *think* we didn't do it for cxxRecordDecl, because
t; I *think* we didn't do it for cxxRecordDecl, because Richard said
>>> >> >> > that's
>>> >> >> > a
>>> >> >> > relic we should get rid of anyway, but I'm not sure.
>>> >> >>
>>> >> >> FWI
; >> >> > relic we should get rid of anyway, but I'm not sure.
>> >> >>
>> >> >> FWIW, I think the state we're in is the worst of all worlds. It's
>> not
>> >> >> intuitive that recordDecl() doesn't match a
et rid of anyway, but I'm not sure.
> >> >>
> >> >> FWIW, I think the state we're in is the worst of all worlds. It's not
> >> >> intuitive that recordDecl() doesn't match a struct in C mode, and as
> >> >> it stands, there
match a struct in C mode, and as
>> >> it stands, there is no way to match a struct or union declaration in C
>> >> at all.
>> >
>> >
>> > Agreed. Best intentions. Worst possible outcome. That's software
>> > development
>> > :)
&
of all worlds. It's not
> >> intuitive that recordDecl() doesn't match a struct in C mode, and as
> >> it stands, there is no way to match a struct or union declaration in C
> >> at all.
> >
> >
> > Agreed. Best intentions. Worst possible outcome
ch a struct or union declaration in C
>> at all.
>
>
> Agreed. Best intentions. Worst possible outcome. That's software development
> :)
>
>> >
>> > On Fri, Sep 4, 2015 at 8:32 PM Aaron Ballman
>> > wrote:
>> >>
>> >>
development :)
>
> > On Fri, Sep 4, 2015 at 8:32 PM Aaron Ballman
> wrote:
> >>
> >> It turns out that the recordDecl() AST matcher doesn't match
> >> RecordDecl objects; instead, it matches CXXRecordDecl objects. This
> >> is... unfortunate... as
ct in C mode, and as
it stands, there is no way to match a struct or union declaration in C
at all.
>
> On Fri, Sep 4, 2015 at 8:32 PM Aaron Ballman wrote:
>>
>> It turns out that the recordDecl() AST matcher doesn't match
>> RecordDecl objects; instead, it matches CX
ard said that's a
relic we should get rid of anyway, but I'm not sure.
On Fri, Sep 4, 2015 at 8:32 PM Aaron Ballman wrote:
> It turns out that the recordDecl() AST matcher doesn't match
> RecordDecl objects; instead, it matches CXXRecordDecl objects. This
> is... unfortun
It turns out that the recordDecl() AST matcher doesn't match
RecordDecl objects; instead, it matches CXXRecordDecl objects. This
is... unfortunate... as it makes writing AST matchers more complicated
because of having to translate between recordDecl()/CXXRecordDecl. It
also makes it impossib
38 matches
Mail list logo
