jdenny created this revision.
jdenny added reviewers: aaron.ballman, rsmith, hfinkel.
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.
https://reviews.llvm.org/D46903
Files:
test/SemaCXX/attr-print.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -374,6 +374,11 @@
OS << Default;
OS << ";";
}
+
+ std::string getIsOmitted() const override {
+ return "Default" + getUpperName().str() + " == get" +
+ getUpperName().str() + "()";
+ }
};
class StringArgument : public Argument {
@@ -1432,6 +1437,7 @@
unsigned NonFakeArgs = 0;
unsigned TrailingOptArgs = 0;
bool FoundNonOptArg = false;
+ std::string Indent = " ";
for (const auto &arg : llvm::reverse(Args)) {
if (arg->isFake())
continue;
@@ -1446,9 +1452,14 @@
}
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 << \"";
@@ -1463,26 +1474,21 @@
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")
+ if (ArgIndex >= NonFakeArgs - TrailingOptArgs) {
OS << "\";\n"
- << " if (!(" << IsOmitted << ")) {\n"
- << " OS << \"";
+ << " if (" << ArgIndex << " < " << NonFakeArgs
+ << " - TrailingOmittedArgs) {\n";
+ if (ArgIndex)
+ OS << " OS << \", \";\n";
+ OS << " OS << \"";
+ } else if (ArgIndex)
+ OS << ", ";
arg->writeValue(OS);
- if (arg->isOptional() && IsOmitted != "false")
+ if (ArgIndex >= NonFakeArgs - TrailingOptArgs) {
OS << "\";\n"
<< " }\n"
<< " OS << \"";
+ }
++ArgIndex;
}
if (TrailingOptArgs < NonFakeArgs)
Index: test/SemaCXX/attr-print.cpp
===================================================================
--- test/SemaCXX/attr-print.cpp
+++ test/SemaCXX/attr-print.cpp
@@ -34,3 +34,27 @@
// 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)));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
