Slightly change the logic of dealing with each type of attributes and add
more test cases :)
For C++11 attributes, they could be printed anywhere except at end of
declaration; for GNU attributes, we enforce that these attributes only appear
at end of declaration to prevent them printing out multiple times; for special
declaration location like empty declaration, we don't check the print
constraints so all types of attributes are printed.
Richard, what's your thoughts on this approach? I think I understand the
principals you mentioned in last review, but I don't know how to apply it to
cases like these so both can be printed out correctly if we only allow C++11
attributes to print on a single location:
[[...]] int x;
struct [[...]] s;
Thanks!
Hi rsmith,
http://llvm-reviews.chandlerc.com/D395
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D395?vs=1315&id=1362#toc
Files:
test/SemaCXX/cxx11-attr-print.cpp
include/clang/AST/Attr.h
utils/TableGen/ClangAttrEmitter.cpp
lib/AST/DeclPrinter.cpp
lib/AST/StmtPrinter.cpp
Index: test/SemaCXX/cxx11-attr-print.cpp
===================================================================
--- test/SemaCXX/cxx11-attr-print.cpp
+++ test/SemaCXX/cxx11-attr-print.cpp
@@ -7,14 +7,14 @@
// CHECK: int y __declspec(align(4));
__declspec(align(4)) int y;
-// CHECK: int z {{\[}}[gnu::aligned(4)]];
+// CHECK: {{\[}}[gnu::aligned(4)]] int z;
int z [[gnu::aligned(4)]];
// CHECK: __attribute__((deprecated("warning")));
int a __attribute__((deprecated("warning")));
-// CHECK: int b {{\[}}[gnu::deprecated("warning")]];
-int b [[gnu::deprecated("warning")]];
+// CHECK: {{\[}}[gnu::deprecated("warning")]] int b __attribute__((unused));
+int b [[gnu::deprecated("warning")]] __attribute__((unused));
// CHECK: int cxx11_alignas alignas(4);
alignas(4) int cxx11_alignas;
@@ -31,34 +31,35 @@
// CHECK: int f1() __attribute__((warn_unused_result));
int f1() __attribute__((warn_unused_result));
-// CHECK: {{\[}}[clang::warn_unused_result]];
+// CHECK: {{\[}}[clang::warn_unused_result]] int f2();
int f2 [[clang::warn_unused_result]] ();
-// CHECK: {{\[}}[gnu::warn_unused_result]];
+// CHECK: {{\[}}[gnu::warn_unused_result]] int f3();
int f3 [[gnu::warn_unused_result]] ();
-// FIXME: ast-print need to print C++11
-// attribute after function declare-id.
-// CHECK: {{\[}}[noreturn]];
+// CHECK: {{\[}}[noreturn]] void f4();
void f4 [[noreturn]] ();
-// CHECK: {{\[}}[std::noreturn]];
+// CHECK: {{\[}}[std::noreturn]] void f5();
void f5 [[std::noreturn]] ();
// CHECK: __attribute__((gnu_inline));
inline void f6() __attribute__((gnu_inline));
-// CHECK: {{\[}}[gnu::gnu_inline]];
+// CHECK: {{\[}}[gnu::gnu_inline]] inline void f7();
inline void f7 [[gnu::gnu_inline]] ();
// arguments printing
// CHECK: __attribute__((format("printf", 2, 3)));
void f8 (void *, const char *, ...) __attribute__ ((format (printf, 2, 3)));
+// CHECK: _Noreturn void f9();
+_Noreturn void f9();
+
// CHECK: int m __attribute__((aligned(4
// CHECK: int n alignas(4
// CHECK: static int f() __attribute__((pure))
-// CHECK: static int g() {{\[}}[gnu::pure]]
+// CHECK: {{\[}}[gnu::pure]] static int g()
template <typename T> struct S {
__attribute__((aligned(4))) int m;
alignas(4) int n;
@@ -73,5 +74,18 @@
// CHECK: int m __attribute__((aligned(4
// CHECK: int n alignas(4
// CHECK: static int f() __attribute__((pure))
-// CHECK: static int g() {{\[}}[gnu::pure]]
+// CHECK: {{\[}}[gnu::pure]] static int g()
template struct S<int>;
+
+// CHECK: {{\[}}[gnu::deprecated("")]] {{\[}}[gnu::pure]] void combo() __attribute__((always_inline));
+void combo [[gnu::pure]] [[gnu::deprecated("")]] () __attribute__((always_inline));
+
+// CHECK: enum {{\[}}[gnu::unused]] enum_attr {
+enum [[gnu::unused]] enum_attr {} __attribute__((deprecated));
+
+// CHECK: class {{\[}}[gnu::unused]] class_attr {
+class [[gnu::unused]] class_attr {} __attribute__((deprecated));
+
+// CHECK: {{\[}}[gnu::unused]] int foooo __attribute__((deprecated(""))) = 0;
+int foooo [[gnu::unused]] __attribute__((deprecated)) = 0;
+
Index: include/clang/AST/Attr.h
===================================================================
--- include/clang/AST/Attr.h
+++ include/clang/AST/Attr.h
@@ -62,6 +62,22 @@
}
public:
+ /// \brief Describes the kind of a syntax location where the attribute
+ /// could appear. It is used to print out the attribute at right location.
+ enum LocationKind {
+ /// \brief Attribute appears at the start of a declaration.
+ LK_StartOfDecl,
+ /// \brief Attribute appears at the end of a declaration.
+ LK_EndOfDecl,
+ /// \brief Attribute appears at an empty declaration.
+ LK_EmptyDecl,
+ /// \brief Attribute appears between class-key and class-head-name.
+ LK_ClassHead,
+ /// \brief Attribute appears at the start of a statement.
+ LK_StartOfStmt
+ };
+
+public:
// Forward so that the regular new and delete do not hide global ones.
void* operator new(size_t Bytes, ASTContext &C,
size_t Alignment = 16) throw() {
@@ -101,7 +117,8 @@
// Pretty print this attribute.
virtual void printPretty(raw_ostream &OS,
- const PrintingPolicy &Policy) const = 0;
+ const PrintingPolicy &Policy,
+ LocationKind LK) const = 0;
};
class InheritableAttr : public Attr {
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -755,7 +755,8 @@
std::vector<Record*> Spellings = R.getValueAsListOfDefs("Spellings");
OS << "void " << R.getName() << "Attr::printPretty("
- << "raw_ostream &OS, const PrintingPolicy &Policy) const {\n";
+ << "raw_ostream &OS, const PrintingPolicy &Policy, "
+ << "LocationKind LK) const {\n";
if (Spellings.size() == 0) {
OS << "}\n\n";
@@ -800,10 +801,26 @@
Spelling += Name;
- OS <<
- " case " << I << " : {\n"
- " OS << \"" + Prefix.str() + Spelling.str();
+ OS << " case " << I << " : {\n";
+ // C++11 attributes should not be printed at end of a declaration,
+ // because in case of a function declaration, attributes at end of
+ // declaration appertain to function types, which we don't support now.
+ if (Variety == "CXX11")
+ OS << " if (LK != LK_EndOfDecl) {\n";
+
+ // C11 _Noreturn attribute should be printed only at the start of
+ // a declaration.
+ if (Variety == "Keyword" && Name == "_Noreturn")
+ OS << " if (LK == LK_StartOfDecl) {\n";
+
+ // GNU attributes appear either at end of declaration, or at an empty
+ // declaration.
+ if (Variety == "GNU")
+ OS << " if (LK == LK_EndOfDecl || LK == LK_EmptyDecl) {\n";
+
+ OS << " OS << \"" + Prefix.str() + Spelling.str();
+
if (Args.size()) OS << "(";
if (Spelling == "availability") {
writeAvailabilityValue(OS);
@@ -817,7 +834,14 @@
if (Args.size()) OS << ")";
OS << Suffix.str() + "\";\n";
+ // Emit a space after printing the attribute unless the attribute is
+ // at end of a declaration.
+ OS << " if (LK != LK_EndOfDecl) OS << \" \";\n";
+ if (Variety == "CXX11" || Variety == "GNU" ||
+ (Variety == "Keyword" && Name == "_Noreturn"))
+ OS << " }\n";
+
OS <<
" break;\n"
" }\n";
@@ -946,7 +970,8 @@
OS << " virtual " << R.getName() << "Attr *clone (ASTContext &C) const;\n";
OS << " virtual void printPretty(raw_ostream &OS,\n"
- << " const PrintingPolicy &Policy) const;\n";
+ << " const PrintingPolicy &Policy,\n"
+ << " LocationKind LC) const;\n";
writeAttrAccessorDefinition(R, OS);
Index: lib/AST/DeclPrinter.cpp
===================================================================
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -85,7 +85,7 @@
void PrintTemplateParameters(const TemplateParameterList *Params,
const TemplateArgumentList *Args = 0);
- void prettyPrintAttributes(Decl *D);
+ void prettyPrintAttributes(Decl *D, Attr::LocationKind LC);
};
}
@@ -183,7 +183,7 @@
return Out;
}
-void DeclPrinter::prettyPrintAttributes(Decl *D) {
+void DeclPrinter::prettyPrintAttributes(Decl *D, Attr::LocationKind LK) {
if (Policy.PolishForDeclaration)
return;
@@ -191,7 +191,7 @@
AttrVec &Attrs = D->getAttrs();
for (AttrVec::const_iterator i=Attrs.begin(), e=Attrs.end(); i!=e; ++i) {
Attr *A = *i;
- A->printPretty(Out, Policy);
+ A->printPretty(Out, Policy, LK);
}
}
}
@@ -328,14 +328,15 @@
}
void DeclPrinter::VisitTypedefDecl(TypedefDecl *D) {
+ prettyPrintAttributes(D, Attr::LK_StartOfDecl);
if (!Policy.SuppressSpecifiers) {
Out << "typedef ";
-
+
if (D->isModulePrivate())
Out << "__module_private__ ";
}
D->getUnderlyingType().print(Out, Policy, D->getName());
- prettyPrintAttributes(D);
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
}
void DeclPrinter::VisitTypeAliasDecl(TypeAliasDecl *D) {
@@ -352,6 +353,9 @@
else
Out << "struct ";
}
+
+ prettyPrintAttributes(D, Attr::LK_ClassHead);
+
Out << *D;
if (D->isFixed())
@@ -362,21 +366,27 @@
VisitDeclContext(D);
Indent() << "}";
}
- prettyPrintAttributes(D);
+
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
}
void DeclPrinter::VisitRecordDecl(RecordDecl *D) {
if (!Policy.SuppressSpecifiers && D->isModulePrivate())
Out << "__module_private__ ";
Out << D->getKindName();
- if (D->getIdentifier())
- Out << ' ' << *D;
+ if (D->getIdentifier()) {
+ Out << ' ';
+ prettyPrintAttributes(D, Attr::LK_ClassHead);
+ Out << *D;
+ }
if (D->isCompleteDefinition()) {
Out << " {\n";
VisitDeclContext(D);
Indent() << "}";
}
+
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
}
void DeclPrinter::VisitEnumConstantDecl(EnumConstantDecl *D) {
@@ -388,6 +398,7 @@
}
void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
+ prettyPrintAttributes(D, Attr::LK_StartOfDecl);
CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
if (!Policy.SuppressSpecifiers) {
switch (D->getStorageClassAsWritten()) {
@@ -554,7 +565,7 @@
Ty.print(Out, Policy, Proto);
}
- prettyPrintAttributes(D);
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
if (D->isPure())
Out << " = 0";
@@ -609,6 +620,7 @@
}
void DeclPrinter::VisitFieldDecl(FieldDecl *D) {
+ prettyPrintAttributes(D, Attr::LK_StartOfDecl);
if (!Policy.SuppressSpecifiers && D->isMutable())
Out << "mutable ";
if (!Policy.SuppressSpecifiers && D->isModulePrivate())
@@ -621,6 +633,8 @@
D->getBitWidth()->printPretty(Out, 0, Policy, Indentation);
}
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
+
Expr *Init = D->getInClassInitializer();
if (!Policy.SuppressInitializers && Init) {
if (D->getInClassInitStyle() == ICIS_ListInit)
@@ -629,7 +643,6 @@
Out << " = ";
Init->printPretty(Out, 0, Policy, Indentation);
}
- prettyPrintAttributes(D);
}
void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
@@ -638,6 +651,7 @@
void DeclPrinter::VisitVarDecl(VarDecl *D) {
+ prettyPrintAttributes(D, Attr::LK_StartOfDecl);
StorageClass SCAsWritten = D->getStorageClassAsWritten();
if (!Policy.SuppressSpecifiers && SCAsWritten != SC_None)
Out << VarDecl::getStorageClassSpecifierString(SCAsWritten) << " ";
@@ -651,6 +665,9 @@
if (ParmVarDecl *Parm = dyn_cast<ParmVarDecl>(D))
T = Parm->getOriginalType();
T.print(Out, Policy, D->getName());
+
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
+
Expr *Init = D->getInit();
if (!Policy.SuppressInitializers && Init) {
bool ImplicitInit = false;
@@ -673,7 +690,6 @@
Out << ")";
}
}
- prettyPrintAttributes(D);
}
void DeclPrinter::VisitParmVarDecl(ParmVarDecl *D) {
@@ -725,15 +741,18 @@
}
void DeclPrinter::VisitEmptyDecl(EmptyDecl *D) {
- prettyPrintAttributes(D);
+ prettyPrintAttributes(D, Attr::LK_EmptyDecl);
}
void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
if (!Policy.SuppressSpecifiers && D->isModulePrivate())
Out << "__module_private__ ";
Out << D->getKindName();
- if (D->getIdentifier())
- Out << ' ' << *D;
+ if (D->getIdentifier()) {
+ Out << ' ';
+ prettyPrintAttributes(D, Attr::LK_ClassHead);
+ Out << *D;
+ }
if (D->isCompleteDefinition()) {
// Print the base classes
@@ -762,6 +781,8 @@
Out << " {\n";
VisitDeclContext(D);
Indent() << "}";
+
+ prettyPrintAttributes(D, Attr::LK_EndOfDecl);
}
}
Index: lib/AST/StmtPrinter.cpp
===================================================================
--- lib/AST/StmtPrinter.cpp
+++ lib/AST/StmtPrinter.cpp
@@ -182,7 +182,7 @@
first = false;
}
// TODO: check this
- (*it)->printPretty(OS, Policy);
+ (*it)->printPretty(OS, Policy, Attr::LK_StartOfStmt);
}
OS << "]] ";
PrintStmt(Node->getSubStmt(), 0);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits