r217771 - When pretty printing attributes that have enumeration arguments, print the enumerator identifier (as a string literal) instead of the internal enumerator integral value.

[Richard Smith]

On Mon, Sep 15, 2014 at 8:14 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> Author: aaronballman
> Date: Mon Sep 15 10:14:13 2014
> New Revision: 217771
>
> URL: http://llvm.org/viewvc/llvm-project?rev=217771&view=rev
> Log:
> When pretty printing attributes that have enumeration arguments, print the
> enumerator identifier (as a string literal) instead of the internal
> enumerator integral value.
>

I don't think this is a complete solution to the problem: EnumArgument is
used both for string literal enumerations and for identifier enumerations,
and the .td file doesn't distinguish the two cases. This will incorrectly
print "" around values for attributes that take an identifier.

Modified:
>     cfe/trunk/test/SemaCXX/attr-print.cpp
>     cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>
> Modified: cfe/trunk/test/SemaCXX/attr-print.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-print.cpp?rev=217771&r1=217770&r2=217771&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/attr-print.cpp (original)
> +++ cfe/trunk/test/SemaCXX/attr-print.cpp Mon Sep 15 10:14:13 2014
> @@ -22,3 +22,12 @@ typedef int Small1 __attribute__((mode(b
>
>  // CHECK: int small __attribute__((mode(byte)));
>  int small __attribute__((mode(byte)));
> +
> +// CHECK: int v __attribute__((visibility("hidden")));
> +int v __attribute__((visibility("hidden")));
> +
> +// FIXME: The attribute should be printed with the tag declaration.
> +class __attribute__((consumable(unknown))) AttrTester1 {
> +  // CHECK: void callableWhen()
> __attribute__((callable_when("unconsumed", "consumed")));
> +  void callableWhen()  __attribute__((callable_when("unconsumed",
> "consumed")));
> +};
>
> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=217771&r1=217770&r2=217771&view=diff
>
> ==============================================================================
> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Mon Sep 15 10:14:13 2014
> @@ -510,6 +510,12 @@ namespace {
>    class VariadicArgument : public Argument {
>      std::string Type, ArgName, ArgSizeName, RangeName;
>
> +  protected:
> +    // Assumed to receive a parameter: raw_ostream OS.
> +    virtual void writeValueImpl(raw_ostream &OS) const {
> +      OS << "    OS << Val;\n";
> +    }
> +
>    public:
>      VariadicArgument(const Record &Arg, StringRef Attr, std::string T)
>          : Argument(Arg, Attr), Type(T), ArgName(getLowerName().str() +
> "_"),
> @@ -589,9 +595,9 @@ namespace {
>        OS << "  bool isFirst = true;\n"
>           << "  for (const auto &Val : " << RangeName << "()) {\n"
>           << "    if (isFirst) isFirst = false;\n"
> -         << "    else OS << \", \";\n"
> -         << "    OS << Val;\n"
> -         << "  }\n";
> +         << "    else OS << \", \";\n";
> +      writeValueImpl(OS);
> +      OS << "  }\n";
>        OS << "  OS << \"";
>      }
>      void writeDump(raw_ostream &OS) const override {
> @@ -678,7 +684,8 @@ namespace {
>        OS << "Record.push_back(SA->get" << getUpperName() << "());\n";
>      }
>      void writeValue(raw_ostream &OS) const override {
> -      OS << "\" << get" << getUpperName() << "() << \"";
> +      OS << "\\\"\" << " << getAttrName() << "Attr::Convert" << type <<
> "ToStr(get"
> +         << getUpperName() << "()) << \"\\\"";
>      }
>      void writeDump(raw_ostream &OS) const override {
>        OS << "    switch(SA->get" << getUpperName() << "()) {\n";
> @@ -703,13 +710,37 @@ namespace {
>        OS << "    if (R) {\n";
>        OS << "      Out = *R;\n      return true;\n    }\n";
>        OS << "    return false;\n";
> -      OS << "  }\n";
> +      OS << "  }\n\n";
> +
> +      // Mapping from enumeration values back to enumeration strings isn't
> +      // trivial because some enumeration values have multiple named
> +      // enumerators, such as type_visibility(internal) and
> +      // type_visibility(hidden) both mapping to
> TypeVisibilityAttr::Hidden.
> +      OS << "  static const char *Convert" << type << "ToStr("
> +         << type << " Val) {\n"
> +         << "    switch(Val) {\n";
> +      std::set<std::string> Uniques;
> +      for (size_t I = 0; I < enums.size(); ++I) {
> +        if (Uniques.insert(enums[I]).second)
> +          OS << "    case " << getAttrName() << "Attr::" << enums[I]
> +             << ": return \"" << values[I] << "\";\n";
> +      }
> +      OS << "    }\n"
> +         << "    llvm_unreachable(\"No enumerator with that value\");\n"
> +         << "  }\n";
>      }
>    };
>
>    class VariadicEnumArgument: public VariadicArgument {
>      std::string type, QualifiedTypeName;
>      std::vector<std::string> values, enums, uniques;
> +
> +  protected:
> +    void writeValueImpl(raw_ostream &OS) const override {
> +      OS << "    OS << \"\\\"\" << " << getAttrName() << "Attr::Convert"
> << type
> +         << "ToStr(Val)" << "<< \"\\\"\";\n";
> +    }
> +
>    public:
>      VariadicEnumArgument(const Record &Arg, StringRef Attr)
>        : VariadicArgument(Arg, Attr, Arg.getValueAsString("Type")),
> @@ -785,7 +816,20 @@ namespace {
>        OS << "    if (R) {\n";
>        OS << "      Out = *R;\n      return true;\n    }\n";
>        OS << "    return false;\n";
> -      OS << "  }\n";
> +      OS << "  }\n\n";
> +
> +      OS << "  static const char *Convert" << type << "ToStr("
> +        << type << " Val) {\n"
> +        << "    switch(Val) {\n";
> +      std::set<std::string> Uniques;
> +      for (size_t I = 0; I < enums.size(); ++I) {
> +        if (Uniques.insert(enums[I]).second)
> +          OS << "    case " << getAttrName() << "Attr::" << enums[I]
> +          << ": return \"" << values[I] << "\";\n";
> +      }
> +      OS << "    }\n"
> +        << "    llvm_unreachable(\"No enumerator with that value\");\n"
> +        << "  }\n";
>      }
>    };
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140915/af11c400/attachment.html>