I suspect you're right about that. Hopefully it would be easy enough. Unfortunately, SourceLocation doesn't have an operator<, which kind of stinks. I suspect that it'll be a non-trivial thing to do, since it is actually possible to have attributes added with empty source-locations in some case, but hopefully whoever is brave enough to tackle this can figure it out.
-----Original Message----- From: Aaron Ballman [mailto:aaron.ball...@gmail.com] Sent: Friday, August 3, 2018 7:35 AM To: Keane, Erich <erich.ke...@intel.com> Cc: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org; Michael Kruse <l...@meinersbur.de>; Hal Finkel <hfin...@anl.gov>; Tyler Nowicki <tnowi...@apple.com>; Alexey Bataev <a.bat...@hotmail.com>; John McCall <rjmcc...@gmail.com>; George Burgess IV <george.burgess...@gmail.com>; Nick Lewycky <nicho...@mxc.ca>; Nick Lewycky <nlewy...@google.com>; d...@google.com; sammcc...@google.com; david.gr...@arm.com; llvm-commits <llvm-comm...@lists.llvm.org>; jrt...@jrtc27.com; Richard Smith <rich...@metafoo.co.uk>; Eric Christopher <echri...@gmail.com>; zhaos...@codeaurora.org; Simon Atanasyan <si...@atanasyan.com>; cfe-commits <cfe-commits@lists.llvm.org>; junb...@codeaurora.org; florian.h...@arm.com Subject: Re: [PATCH] D48100: Append new attributes to the end of an AttributeList. On Fri, Aug 3, 2018 at 10:09 AM, Keane, Erich <erich.ke...@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.ball...@gmail.com] > Sent: Friday, August 3, 2018 7:02 AM > To: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org > Cc: Michael Kruse <l...@meinersbur.de>; Hal Finkel <hfin...@anl.gov>; > Tyler Nowicki <tnowi...@apple.com>; Alexey Bataev > <a.bat...@hotmail.com>; John McCall <rjmcc...@gmail.com>; George > Burgess IV <george.burgess...@gmail.com>; Nick Lewycky > <nicho...@mxc.ca>; Nick Lewycky <nlewy...@google.com>; d...@google.com; > sammcc...@google.com; david.gr...@arm.com; llvm-commits > <llvm-comm...@lists.llvm.org>; jrt...@jrtc27.com; Richard Smith > <rich...@metafoo.co.uk>; Keane, Erich <erich.ke...@intel.com>; Eric > Christopher <echri...@gmail.com>; zhaos...@codeaurora.org; Simon > Atanasyan <si...@atanasyan.com>; cfe-commits > <cfe-commits@lists.llvm.org>; junb...@codeaurora.org; > florian.h...@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 > <revi...@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 >> >> >> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
