r201246 - Attributes: Emit enumerators in td file declaration order

Aaron Ballman aaron at aaronballman.com
Wed Feb 12 11:24:19 PST 2014


On Wed, Feb 12, 2014 at 2:16 PM, Reid Kleckner <rnk at google.com> wrote:
> On Wed, Feb 12, 2014 at 10:40 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Thanks for taking care of this! A suggestion below.
>>
>> On Wed, Feb 12, 2014 at 1:22 PM, Reid Kleckner <reid at kleckner.net> wrote:
>> > Author: rnk
>> > Date: Wed Feb 12 12:22:18 2014
>> > New Revision: 201246
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=201246&view=rev
>> > Log:
>> > Attributes: Emit enumerators in td file declaration order
>> >
>> > Modified:
>> >     cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>> >
>> > Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=201246&r1=201245&r2=201246&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
>> > +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Wed Feb 12 12:22:18
>> > 2014
>> > @@ -606,6 +606,22 @@ namespace {
>> >      }
>> >    };
>> >
>> > +  // Unique the enums, but maintain the original declaration ordering.
>> > +  std::vector<std::string> uniqueEnumsInOrder(std::vector<std::string>
>> > enums) {
>>
>> Can this take a const & instead of a value?
>
>
> Ah, yeah, oversight.
>
>>
>> > +    std::vector<std::string> uniques;
>> > +    std::set<std::string> unique_set(enums.begin(), enums.end());
>> > +    for (std::vector<std::string>::const_iterator i = enums.begin(),
>> > +                                                  e = enums.end();
>> > +         i != e; ++i) {
>> > +      std::set<std::string>::iterator set_i = unique_set.find(*i);
>> > +      if (set_i != unique_set.end()) {
>> > +        uniques.push_back(*i);
>> > +        unique_set.erase(set_i);
>> > +      }
>> > +    }
>> > +    return uniques;
>> > +  }
>> > +
>>
>> This is doing more work than it needs to -- the std::set already
>> uniques items in its constructor. So this could be written as:
>>
>> static std::vector<std::string> uniqueEnumsInOrder(const
>> std::vector<std::string> &Enums) {
>>   std::set<std::string> Uniques(Enums.begin(), Enums.end());
>>   return std::vector<std::string>(Uniques.begin(), Uniques.end());
>> }
>
>
> std::set is an rbtree that will implicitly sort the strings, so I don't
> think that will work.
>
> The most declarative way I could think to write this was something like:
>
> // decorate strings with an index
> vector<pair<string, int>>> decorated(...);
> // sort unique in the usual way with a custom comparator
> std::sort(decorated, dotFirst);
> std::unique(..., dotFirst);
> // resort with dotSecond
> std::sort(decorated, dotSecond);
> // apply dotFirst to extract the strings
>
> ... but it didn't seem worth it.

Ah shoot, that's a good point.

~Aaron



More information about the cfe-commits mailing list