[PATCH] D48100: Append new attributes to the end of an AttributeList.

[Aaron Ballman via cfe-commits]

On Fri, Aug 3, 2018 at 10:09 AM, Keane, Erich <erich.keane at intel.com> wrote:
>> As a possible idea (that may or may not work): the Attr object itself has a SourceRange on it; perhaps a solution is to keep the > attributes in sorted order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> Interestingly, I think I came up with that idea in a comment on D50214.  I think that we should either keep the attributes sorted, or make the iterator give a sorted version.

Oh, hey, so you did! I think keeping them in a sorted order is
preferable to having the iterator return a sorted version; I believe
we iterate attribute more often than we add them because things like
hasAttr<>() and getAttr<>() both rely on iterating through the
attributes.

~Aaron

>
>
>
> -----Original Message-----
> From: Aaron Ballman [mailto:aaron.ballman at gmail.com]
> Sent: Friday, August 3, 2018 7:02 AM
> To: reviews+D48100+public+bdf72fdc7f8c944e at reviews.llvm.org
> Cc: Michael Kruse <llvm at meinersbur.de>; Hal Finkel <hfinkel at anl.gov>; Tyler Nowicki <tnowicki at apple.com>; Alexey Bataev <a.bataev at hotmail.com>; John McCall <rjmccall at gmail.com>; George Burgess IV <george.burgess.iv at gmail.com>; Nick Lewycky <nicholas at mxc.ca>; Nick Lewycky <nlewycky at google.com>; dlj at google.com; sammccall at google.com; david.green at arm.com; llvm-commits <llvm-commits at lists.llvm.org>; jrtc27 at jrtc27.com; Richard Smith <richard at metafoo.co.uk>; Keane, Erich <erich.keane at intel.com>; Eric Christopher <echristo at gmail.com>; zhaoshiz at codeaurora.org; Simon Atanasyan <simon at atanasyan.com>; cfe-commits <cfe-commits at lists.llvm.org>; junbuml at codeaurora.org; florian.hahn at arm.com
> Subject: Re: [PATCH] D48100: Append new attributes to the end of an AttributeList.
>
> On Fri, Aug 3, 2018 at 8:53 AM, Erich Keane via Phabricator <reviews at reviews.llvm.org> wrote:
>> erichkeane added a comment.
>>
>> In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:
>>
>>> I have two approaches to tackle the wrong marker order: https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO both are too invasive to be justified for the small issue.
>>
>>
>> I think you're right here.  I despise https://reviews.llvm.org/D50215, and only mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer leaving this as a "FIXME" somewhere.
>
> Oye, I'm in agreement that this should be fixed but that neither of these approaches leaves me feeling warm and fuzzy.
>
> As a possible idea (that may or may not work): the Attr object itself has a SourceRange on it; perhaps a solution is to keep the attributes in sorted order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> ~Aaron
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D48100
>>
>>
>>