jdenny updated this revision to Diff 135982.
jdenny added a comment.
Aaron: Because the last two arguments for objc_bridge_related must be delimited
with commas, this revision takes the view that they are not optional but are
permitted to be the empty string.
The test suite behaves as expected. Are there any untested attributes that
might be broken by this patch? Hopefully they can be addressed in a similar
manner.
https://reviews.llvm.org/D43748
Files:
include/clang/Basic/Attr.td
lib/Parse/ParseDecl.cpp
test/Sema/attr-print.c
test/Sema/attr-print.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -231,6 +231,7 @@
virtual void writePCHReadArgs(raw_ostream &OS) const = 0;
virtual void writePCHReadDecls(raw_ostream &OS) const = 0;
virtual void writePCHWrite(raw_ostream &OS) const = 0;
+ virtual std::string getIsOmitted() const { return "false"; }
virtual void writeValue(raw_ostream &OS) const = 0;
virtual void writeDump(raw_ostream &OS) const = 0;
virtual void writeDumpChildren(raw_ostream &OS) const {}
@@ -298,33 +299,43 @@
std::string(getUpperName()) + "()");
}
+ std::string getIsOmitted() const override {
+ if (type == "FunctionDecl *")
+ return "false";
+ if (type == "IdentifierInfo *")
+ return "!get" + getUpperName().str() + "()";
+ if (type == "TypeSourceInfo *")
+ return "false";
+ // FIXME: Do this declaratively in Attr.td.
+ if (getAttrName() == "AllocSize")
+ return "0 == get" + getUpperName().str() + "()";
+ return "false";
+ }
+
void writeValue(raw_ostream &OS) const override {
- if (type == "FunctionDecl *") {
+ if (type == "FunctionDecl *")
OS << "\" << get" << getUpperName()
<< "()->getNameInfo().getAsString() << \"";
- } else if (type == "IdentifierInfo *") {
- OS << "\";\n";
- if (isOptional())
- OS << " if (get" << getUpperName() << "()) ";
- else
- OS << " ";
- OS << "OS << get" << getUpperName() << "()->getName();\n";
- OS << " OS << \"";
- } else if (type == "TypeSourceInfo *") {
+ else if (type == "IdentifierInfo *")
+ // Some non-optional (comma required) identifier arguments can be the
+ // empty string but are then recorded as a nullptr.
+ OS << "\" << (get" << getUpperName() << "() ? get" << getUpperName()
+ << "()->getName() : \"\") << \"";
+ else if (type == "TypeSourceInfo *")
OS << "\" << get" << getUpperName() << "().getAsString() << \"";
- } else {
+ else
OS << "\" << get" << getUpperName() << "() << \"";
- }
}
void writeDump(raw_ostream &OS) const override {
if (type == "FunctionDecl *" || type == "NamedDecl *") {
OS << " OS << \" \";\n";
OS << " dumpBareDeclRef(SA->get" << getUpperName() << "());\n";
} else if (type == "IdentifierInfo *") {
- if (isOptional())
- OS << " if (SA->get" << getUpperName() << "())\n ";
- OS << " OS << \" \" << SA->get" << getUpperName()
+ // Some non-optional (comma required) identifier arguments can be the
+ // empty string but are then recorded as a nullptr.
+ OS << " if (SA->get" << getUpperName() << "())\n"
+ << " OS << \" \" << SA->get" << getUpperName()
<< "()->getName();\n";
} else if (type == "TypeSourceInfo *") {
OS << " OS << \" \" << SA->get" << getUpperName()
@@ -576,12 +587,15 @@
<< "Type());\n";
}
+ std::string getIsOmitted() const override {
+ return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
+ + "Expr";
+ }
+
void writeValue(raw_ostream &OS) const override {
OS << "\";\n";
- // The aligned attribute argument expression is optional.
- OS << " if (is" << getLowerName() << "Expr && "
- << getLowerName() << "Expr)\n";
- OS << " " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+ OS << " " << getLowerName()
+ << "Expr->printPretty(OS, nullptr, Policy);\n";
OS << " OS << \"";
}
@@ -1376,33 +1390,83 @@
continue;
}
- // Fake arguments aren't part of the parsed form and should not be
- // pretty-printed.
- bool hasNonFakeArgs = llvm::any_of(
- Args, [](const std::unique_ptr<Argument> &A) { return !A->isFake(); });
-
- // FIXME: always printing the parenthesis isn't the correct behavior for
- // attributes which have optional arguments that were not provided. For
- // instance: __attribute__((aligned)) will be pretty printed as
- // __attribute__((aligned())). The logic should check whether there is only
- // a single argument, and if it is optional, whether it has been provided.
- if (hasNonFakeArgs)
- OS << "(";
if (Spelling == "availability") {
+ OS << "(";
writeAvailabilityValue(OS);
+ OS << ")";
} else if (Spelling == "deprecated" || Spelling == "gnu::deprecated") {
- writeDeprecatedAttrValue(OS, Variety);
+ OS << "(";
+ writeDeprecatedAttrValue(OS, Variety);
+ OS << ")";
} else {
- unsigned index = 0;
+ // 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;
+ 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";
+ }
+ 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 (index++) OS << ", ";
+ 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 << \"";
arg->writeValue(OS);
+ if (arg->isOptional() && IsOmitted != "false")
+ OS << "\";\n"
+ << " }\n"
+ << " OS << \"";
+ ++ArgIndex;
}
+ if (TrailingOptArgs < NonFakeArgs)
+ OS << ")";
+ else if (TrailingOptArgs)
+ OS << "\";\n"
+ << " if (TrailingOmittedArgs < " << NonFakeArgs << ")\n"
+ << " OS << \")\";\n"
+ << " OS << \"";
}
- if (hasNonFakeArgs)
- OS << ")";
OS << Suffix + "\";\n";
OS <<
Index: test/Sema/attr-print.cpp
===================================================================
--- /dev/null
+++ test/Sema/attr-print.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -ast-print | FileCheck %s
+
+// CHECK: void *as2(int, int) __attribute__((alloc_size(1, 2)));
+void *as2(int, int) __attribute__((alloc_size(1, 2)));
+// CHECK: void *as1(void *, int) __attribute__((alloc_size(2)));
+void *as1(void *, int) __attribute__((alloc_size(2)));
Index: test/Sema/attr-print.c
===================================================================
--- test/Sema/attr-print.c
+++ test/Sema/attr-print.c
@@ -7,6 +7,9 @@
// CHECK: int y __declspec(align(4));
__declspec(align(4)) int y;
+// CHECK: short arr[3] __attribute__((aligned));
+short arr[3] __attribute__((aligned));
+
// CHECK: void foo() __attribute__((const));
void foo() __attribute__((const));
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -1258,7 +1258,9 @@
return;
}
- // Parse optional class method name.
+ // Parse class method name. It's non-optional in the sense that a trailing
+ // comma is required, but it can be the empty string, and then we record a
+ // nullptr.
IdentifierLoc *ClassMethod = nullptr;
if (Tok.is(tok::identifier)) {
ClassMethod = ParseIdentifierLoc();
@@ -1277,7 +1279,8 @@
return;
}
- // Parse optional instance method name.
+ // Parse instance method name. Also non-optional but empty string is
+ // permitted.
IdentifierLoc *InstanceMethod = nullptr;
if (Tok.is(tok::identifier))
InstanceMethod = ParseIdentifierLoc();
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1506,8 +1506,8 @@
let Spellings = [Clang<"objc_bridge_related">];
let Subjects = SubjectList<[Record], ErrorDiag>;
let Args = [IdentifierArgument<"RelatedClass">,
- IdentifierArgument<"ClassMethod", 1>,
- IdentifierArgument<"InstanceMethod", 1>];
+ IdentifierArgument<"ClassMethod">,
+ IdentifierArgument<"InstanceMethod">];
let HasCustomParsing = 1;
let Documentation = [Undocumented];
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
