On Wed, Feb 12, 2014 at 2:16 PM, Reid Kleckner <[email protected]> wrote: > 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.
Ah shoot, that's a good point. ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
