r201246 - Attributes: Emit enumerators in td file declaration order

[Reid Kleckner]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140212/638d43e2/attachment.html>