On Wed, Feb 12, 2014 at 10:40 AM, Aaron Ballman <[email protected]>wrote:
> Thanks for taking care of this! A suggestion below. > > On Wed, Feb 12, 2014 at 1:22 PM, Reid Kleckner <[email protected]> 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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
