r201246 - Attributes: Emit enumerators in td file declaration order

Aaron Ballman aaron at aaronballman.com
Wed Feb 12 10:40:06 PST 2014


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?

> +    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());
}

>    class EnumArgument : public Argument {
>      std::string type;
>      std::vector<std::string> values, enums, uniques;
> @@ -614,11 +630,8 @@ namespace {
>        : Argument(Arg, Attr), type(Arg.getValueAsString("Type")),
>          values(Arg.getValueAsListOfStrings("Values")),
>          enums(Arg.getValueAsListOfStrings("Enums")),
> -        uniques(enums)
> +        uniques(uniqueEnumsInOrder(enums))
>      {
> -      // Calculate the various enum values
> -      std::sort(uniques.begin(), uniques.end());
> -      uniques.erase(std::unique(uniques.begin(), uniques.end()), uniques.end());
>        // FIXME: Emit a proper error
>        assert(!uniques.empty());
>      }
> @@ -711,12 +724,8 @@ namespace {
>          type(Arg.getValueAsString("Type")),
>          values(Arg.getValueAsListOfStrings("Values")),
>          enums(Arg.getValueAsListOfStrings("Enums")),
> -        uniques(enums)
> +        uniques(uniqueEnumsInOrder(enums))
>      {
> -      // Calculate the various enum values
> -      std::sort(uniques.begin(), uniques.end());
> -      uniques.erase(std::unique(uniques.begin(), uniques.end()), uniques.end());
> -
>        QualifiedTypeName = getAttrName().str() + "Attr::" + type;
>
>        // FIXME: Emit a proper error

~Aaron



More information about the cfe-commits mailing list