r326266 - Improve the way attribute argument printing happens for omitted optional arguments when pretty printing.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 27 15:49:28 PST 2018
Author: aaronballman
Date: Tue Feb 27 15:49:28 2018
New Revision: 326266
URL: http://llvm.org/viewvc/llvm-project?rev=326266&view=rev
Log:
Improve the way attribute argument printing happens for omitted optional arguments when pretty printing.
Patch by Joel Denny.
Added:
cfe/trunk/test/Sema/attr-print.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/test/Sema/attr-print.c
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=326266&r1=326265&r2=326266&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 27 15:49:28 2018
@@ -1506,8 +1506,8 @@ def ObjCBridgeRelated : InheritableAttr
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];
}
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=326266&r1=326265&r2=326266&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Feb 27 15:49:28 2018
@@ -1258,7 +1258,9 @@ void Parser::ParseObjCBridgeRelatedAttri
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 @@ void Parser::ParseObjCBridgeRelatedAttri
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();
Modified: cfe/trunk/test/Sema/attr-print.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-print.c?rev=326266&r1=326265&r2=326266&view=diff
==============================================================================
--- cfe/trunk/test/Sema/attr-print.c (original)
+++ cfe/trunk/test/Sema/attr-print.c Tue Feb 27 15:49:28 2018
@@ -7,6 +7,9 @@ int x __attribute__((aligned(4)));
// 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));
Added: cfe/trunk/test/Sema/attr-print.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-print.cpp?rev=326266&view=auto
==============================================================================
--- cfe/trunk/test/Sema/attr-print.cpp (added)
+++ cfe/trunk/test/Sema/attr-print.cpp Tue Feb 27 15:49:28 2018
@@ -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)));
Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=326266&r1=326265&r2=326266&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue Feb 27 15:49:28 2018
@@ -231,6 +231,7 @@ namespace {
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,23 +299,28 @@ namespace {
std::string(getUpperName()) + "()");
}
+ std::string getIsOmitted() const override {
+ if (type == "IdentifierInfo *")
+ return "!get" + getUpperName().str() + "()";
+ // 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 {
@@ -322,9 +328,10 @@ namespace {
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 +583,15 @@ namespace {
<< "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 +1386,83 @@ writePrettyPrintFunction(Record &R,
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 <<
More information about the cfe-commits
mailing list