[Attr] Don't print attr arg with default value

[jdenny]

Summary

For example, given:

class __single_inheritance T;

-ast-print -fms-extensions used to print:

class __single_inheritance(1) T;

Clang fails to parse that because the "(1)" is not valid syntax.

This was discovered at:

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180514/228390.html

To fix that, this patch generally suppresses printing any attribute
argument that has its default value unless other arguments will be
printed after it. Doing so should always maintain the original source
semantics and produce valid syntax.

However, doing so also drops arguments where attributes are legally
specified explicitly with their default values but are not followed by
other arguments. Correct syntax in the previous case seems more
important than exact textual fidelity in this case.

[jdenny]

I occurs to me now that I could have fixed __single_inheritance by declaring its argument "fake". However, I still prefer the above solution because (1) it should help with any other attributes where arguments should have been declared fake (but no, I haven't identified any yet), and (2) it generally makes attribute printing more succinct, including cases where originally implicit values were printing as explicit. Again, the sacrifice is that some explicit values now print as implicit, but at least the semantics don't change.

If others disagree, I can change this.

[jdenny]

After further thought, this patch doesn't seem worthwhile. Sorry for all the noise.

test/SemaCXX/attr-print.cpp

 // CHECK: char *PR24565() __attribute__((malloc))
 char *PR24565() __attribute__((__malloc__));
 
 // CHECK: class __attribute__((consumable("unknown"))) AttrTester1
 class __attribute__((consumable(unknown))) AttrTester1 {
   // CHECK: void callableWhen() __attribute__((callable_when("unconsumed", "consumed")));
   void callableWhen()  __attribute__((callable_when("unconsumed", "consumed")));
 };
+
+// CHECK: class __single_inheritance SingleInheritance;
+class __single_inheritance SingleInheritance;
+
+// CHECK: void sentinel(int a, ...) __attribute__((sentinel));
+void sentinel(int a, ...) __attribute__((sentinel));
+
+// CHECK: void sentinel_1n(int a, ...) __attribute__((sentinel(1)));
+void sentinel_1n(int a, ...) __attribute__((sentinel(1)));
+
+// CHECK: void sentinel_1d2n(int a, ...) __attribute__((sentinel(0, 1)));
+void sentinel_1d2n(int a, ...) __attribute__((sentinel(0, 1)));
+
+// CHECK: void sentinel_1n2n(int a, ...) __attribute__((sentinel(1, 1)));
+void sentinel_1n2n(int a, ...) __attribute__((sentinel(1, 1)));
+
+// It would be better if we didn't lose arguments in the following cases, but
+// at least the default values have the same semantics.
+
+// CHECK: void sentinel_1d(int a, ...) __attribute__((sentinel));
+void sentinel_1d(int a, ...) __attribute__((sentinel(0)));
+
+// CHECK: void sentinel_1d2d(int a, ...) __attribute__((sentinel));
+void sentinel_1d2d(int a, ...) __attribute__((sentinel(0, 0)));

utils/TableGen/ClangAttrEmitter.cpp

void writeAccessors(raw_ostream &OS) const override {
       OS << "\n\n  static const " << getType() << " Default" << getUpperName()
          << " = ";
       if (getType() == "bool")
         OS << (Default != 0 ? "true" : "false");
       else
         OS << Default;
       OS << ";";
     }
+
+    std::string getIsOmitted() const override {
+      return "Default" + getUpperName().str() + " == get" +
+             getUpperName().str() + "()";
+    }
   };
 
   class StringArgument : public Argument {
   public:
     StringArgument(const Record &Arg, StringRef Attr)
       : Argument(Arg, Attr)
     {}
 
if (Spelling == "availability") {
       OS << ")";
     } else {
       // To avoid printing parentheses around an empty argument list or
       // printing spurious commas at the end of an argument list, we need to
       // determine where the last provided non-fake argument is.
       unsigned NonFakeArgs = 0;
       unsigned TrailingOptArgs = 0;
       bool FoundNonOptArg = false;
+      std::string Indent = "    ";
       for (const auto &arg : llvm::reverse(Args)) {
         if (arg->isFake())
           continue;
         ++NonFakeArgs;
         if (FoundNonOptArg)
           continue;
         // FIXME: arg->getIsOmitted() == "false" means we haven't implemented
         // any way to detect whether the argument was omitted.
         if (!arg->isOptional() || arg->getIsOmitted() == "false") {
           FoundNonOptArg = true;
           continue;
         }
         if (!TrailingOptArgs++)
           OS << "\";\n"
-             << "    unsigned TrailingOmittedArgs = 0;\n";
-        OS << "    if (" << arg->getIsOmitted() << ")\n"
-           << "      ++TrailingOmittedArgs;\n";
+             << Indent << "unsigned TrailingOmittedArgs = 0;\n";
+        OS << Indent << "if (" << arg->getIsOmitted() << ") {\n";
+        Indent += "  ";
+        OS << Indent << "++TrailingOmittedArgs;\n";
+      }
+      for (unsigned i = 0; i < TrailingOptArgs; ++i) {
+        Indent.resize(Indent.size() - 2);
+        OS << Indent << "}\n";
       }
       if (TrailingOptArgs)
         OS << "    OS << \"";
       if (TrailingOptArgs < NonFakeArgs)
         OS << "(";
       else if (TrailingOptArgs)
         OS << "\";\n"
            << "    if (TrailingOmittedArgs < " << NonFakeArgs << ")\n"
            << "       OS << \"(\";\n"
            << "    OS << \"";
       unsigned ArgIndex = 0;
       for (const auto &arg : Args) {
         if (arg->isFake())
           continue;
-        if (ArgIndex) {
-          if (ArgIndex >= NonFakeArgs - TrailingOptArgs)
-            OS << "\";\n"
-               << "    if (" << ArgIndex << " < " << NonFakeArgs
-               << " - TrailingOmittedArgs)\n"
-               << "      OS << \", \";\n"
-               << "    OS << \"";
-          else
-            OS << ", ";
-        }
-        std::string IsOmitted = arg->getIsOmitted();
-        if (arg->isOptional() && IsOmitted != "false")
-          OS << "\";\n"
-             << "    if (!(" << IsOmitted << ")) {\n"
-             << "      OS << \"";
+        if (ArgIndex >= NonFakeArgs - TrailingOptArgs) {
+             << " - TrailingOmittedArgs) {\n";
+          if (ArgIndex)
+            OS << "      OS << \", \";\n";
+          OS << "      OS << \"";
+        } else if (ArgIndex)
         arg->writeValue(OS);
-        if (arg->isOptional() && IsOmitted != "false")
+        if (ArgIndex >= NonFakeArgs - TrailingOptArgs) {
           OS << "\";\n"
              << "    }\n"
              << "    OS << \"";
+        }
         ++ArgIndex;
       }
       if (TrailingOptArgs < NonFakeArgs)
         OS << ")";
       else if (TrailingOptArgs)
         OS << "\";\n"
            << "    if (TrailingOmittedArgs < " << NonFakeArgs << ")\n"
            << "       OS << \")\";\n"