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

Keane, Erich via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 3 07:09:51 PDT 2018


> 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.  



-----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
>
>
>


More information about the cfe-commits mailing list