Skip to content

Commit a72cf87

Browse files
vgvassilevvgvassilev
committed
Rework the printing of attributes.
Commit 46f3ade introduced a notion of printing the attributes on the left to improve the printing of attributes attached to variable declarations. The intent was to produce more GCC compatible code because clang tends to print the attributes on the right hand side which is not accepted by gcc. This approach has increased the complexity in tablegen and the attrubutes themselves as now the are supposed to know where they could appear. That lead to mishandling of the `override` keyword which is modelled as an attribute in clang. This patch takes an inspiration from the existing approach and tries to keep the position of the attributes as they were written. To do so we use simpler heuristic which checks if the source locations of the attribute precedes the declaration. If so, it is considered to be printed before the declaration. Fixes #87151
1 parent e7e2e90 commit a72cf87

14 files changed

+107
-31
lines changed

‎clang/lib/AST/DeclPrinter.cpp

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "clang/AST/ExprCXX.h"
2222
#include "clang/AST/PrettyPrinter.h"
2323
#include "clang/Basic/Module.h"
24+
#include "clang/Basic/SourceManager.h"
2425
#include "llvm/Support/raw_ostream.h"
2526
using namespace clang;
2627

@@ -117,7 +118,10 @@ namespace {
117118
const TemplateParameterList *Params);
118119
void printTemplateArguments(llvm::ArrayRef<TemplateArgumentLoc> Args,
119120
const TemplateParameterList *Params);
120-
void prettyPrintAttributes(Decl *D);
121+
enum class AttrPosAsWritten { Unknown = 0, Default, Left, Right };
122+
void
123+
prettyPrintAttributes(const Decl *D,
124+
AttrPosAsWritten Pos = AttrPosAsWritten::Default);
121125
void prettyPrintPragmas(Decl *D);
122126
void printDeclType(QualType T, StringRef DeclName, bool Pack = false);
123127
};
@@ -234,12 +238,28 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
234238
return Out;
235239
}
236240

237-
void DeclPrinter::prettyPrintAttributes(Decl *D) {
241+
static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A,
242+
const Decl *D) {
243+
SourceLocation ALoc = A->getLoc();
244+
SourceLocation DLoc = D->getLocation();
245+
const ASTContext &C = D->getASTContext();
246+
if (ALoc.isInvalid() || DLoc.isInvalid())
247+
return DeclPrinter::AttrPosAsWritten::Unknown;
248+
249+
if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
250+
return DeclPrinter::AttrPosAsWritten::Left;
251+
252+
return DeclPrinter::AttrPosAsWritten::Right;
253+
}
254+
255+
void DeclPrinter::prettyPrintAttributes(const Decl *D,
256+
AttrPosAsWritten Pos /*=Default*/) {
238257
if (Policy.PolishForDeclaration)
239258
return;
240259

241260
if (D->hasAttrs()) {
242-
AttrVec &Attrs = D->getAttrs();
261+
assert(Pos != AttrPosAsWritten::Unknown && "Use Default");
262+
const AttrVec &Attrs = D->getAttrs();
243263
for (auto *A : Attrs) {
244264
if (A->isInherited() || A->isImplicit())
245265
continue;
@@ -249,7 +269,20 @@ void DeclPrinter::prettyPrintAttributes(Decl *D) {
249269
#include "clang/Basic/AttrList.inc"
250270
break;
251271
default:
252-
A->printPretty(Out, Policy);
272+
AttrPosAsWritten APos = getPosAsWritten(A, D);
273+
// Might trigger on programatically created attributes or declarations
274+
// with no source locations.
275+
assert(APos != AttrPosAsWritten::Unknown &&
276+
"Invalid source location for attribute or decl.");
277+
assert(APos != AttrPosAsWritten::Default &&
278+
"Default not a valid for an attribute location");
279+
if (Pos == AttrPosAsWritten::Default || Pos == APos) {
280+
if (Pos != AttrPosAsWritten::Left)
281+
Out << ' ';
282+
A->printPretty(Out, Policy);
283+
if (Pos == AttrPosAsWritten::Left)
284+
Out << ' ';
285+
}
253286
break;
254287
}
255288
}
@@ -612,8 +645,10 @@ static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
612645

613646
void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
614647
if (!D->getDescribedFunctionTemplate() &&
615-
!D->isFunctionTemplateSpecialization())
648+
!D->isFunctionTemplateSpecialization()) {
616649
prettyPrintPragmas(D);
650+
prettyPrintAttributes(D, AttrPosAsWritten::Left);
651+
}
617652

618653
if (D->isFunctionTemplateSpecialization())
619654
Out << "template<> ";
@@ -788,7 +823,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
788823
Ty.print(Out, Policy, Proto);
789824
}
790825

791-
prettyPrintAttributes(D);
826+
prettyPrintAttributes(D, AttrPosAsWritten::Right);
792827

793828
if (D->isPureVirtual())
794829
Out << " = 0";
@@ -881,6 +916,8 @@ void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
881916
void DeclPrinter::VisitVarDecl(VarDecl *D) {
882917
prettyPrintPragmas(D);
883918

919+
prettyPrintAttributes(D, AttrPosAsWritten::Left);
920+
884921
if (const auto *Param = dyn_cast<ParmVarDecl>(D);
885922
Param && Param->isExplicitObjectParameter())
886923
Out << "this ";
@@ -926,6 +963,8 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
926963
? D->getIdentifier()->deuglifiedName()
927964
: D->getName());
928965

966+
prettyPrintAttributes(D, AttrPosAsWritten::Right);
967+
929968
Expr *Init = D->getInit();
930969
if (!Policy.SuppressInitializers && Init) {
931970
bool ImplicitInit = false;
@@ -954,7 +993,6 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
954993
Out << ")";
955994
}
956995
}
957-
prettyPrintAttributes(D);
958996
}
959997

960998
void DeclPrinter::VisitParmVarDecl(ParmVarDecl *D) {

‎clang/lib/AST/StmtPrinter.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,11 @@ void StmtPrinter::VisitLabelStmt(LabelStmt *Node) {
292292
}
293293

294294
void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
295-
for (const auto *Attr : Node->getAttrs()) {
295+
llvm::ArrayRef<const Attr *> Attrs = Node->getAttrs();
296+
for (const auto *Attr : Attrs) {
296297
Attr->printPretty(OS, Policy);
298+
if (Attr != Attrs.back())
299+
OS << ' ';
297300
}
298301

299302
PrintStmt(Node->getSubStmt(), 0);

‎clang/test/AST/ast-print-attr-knr.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// This file contain tests for attribute arguments on K&R functions.
2+
3+
// RUN: %clang_cc1 -ast-print -x c -std=c89 -fms-extensions %s -o - | FileCheck %s
4+
5+
// CHECK: int knr(i)
6+
// CHECK-NEXT: int i __attribute__((unused));
7+
// CHECK-NEXT: {
8+
// CHECK-NEXT: return 0;
9+
// CHECK-NEXT: }
10+
int knr(i) int i __attribute__((unused)); { return 0; }
11+
12+
// CHECK: __attribute__((unused)) int knr2(i)
13+
// CHECK-NEXT: int i;
14+
// CHECK-NEXT: {
15+
// CHECK-NEXT: return 0;
16+
// CHECK-NEXT: }
17+
__attribute__((unused)) int knr2(i) int i; { return 0; }

‎clang/test/AST/ast-print-attr.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,9 @@ int *fun_returns() __attribute__((ownership_returns(fun_returns)));
3232

3333
// CHECK: void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));
3434
void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));
35+
36+
// CHECK: int fun_var_unused() {
37+
// CHECK-NEXT: int x __attribute__((unused)) = 0;
38+
// CHECK-NEXT: return x;
39+
// CHECK-NEXT: }
40+
int fun_var_unused() { int x __attribute__((unused)) = 0; return x; }

‎clang/test/AST/ast-print-no-sanitize.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ void should_not_crash_1() __attribute__((no_sanitize_memory));
44
[[clang::no_sanitize_memory]] void should_not_crash_2();
55

66
// CHECK: void should_not_crash_1() __attribute__((no_sanitize("memory")));
7-
// CHECK: void should_not_crash_2() {{\[\[}}clang::no_sanitize("memory"){{\]\]}};
7+
// CHECK: {{\[\[}}clang::no_sanitize("memory"){{\]\]}} void should_not_crash_2();

‎clang/test/AST/ast-print-pragmas.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ void test_templates(int *List, int Length) {
9393
#ifdef MS_EXT
9494
#pragma init_seg(compiler)
9595
// MS-EXT: #pragma init_seg (.CRT$XCC){{$}}
96-
// MS-EXT-NEXT: int x = 3 __declspec(thread);
97-
int __declspec(thread) x = 3;
96+
// MS-EXT-NEXT: __declspec(thread) int x = 3;
97+
__declspec(thread) int x = 3;
9898
#endif //MS_EXT
9999

‎clang/test/AST/attr-print-emit.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,18 @@ class C {
7373
// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
7474
void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
7575
};
76+
77+
#define ANNOTATE_ATTR __attribute__((annotate("Annotated")))
78+
ANNOTATE_ATTR int annotated_attr ANNOTATE_ATTR = 0;
79+
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr __attribute__((annotate("Annotated"))) = 0;
80+
81+
// FIXME: We do not print the attribute as written after the type specifier.
82+
int ANNOTATE_ATTR annotated_attr_fixme = 0;
83+
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr_fixme = 0;
84+
85+
#define NONNULL_ATTR __attribute__((nonnull(1)))
86+
ANNOTATE_ATTR NONNULL_ATTR void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));
87+
// CHECK:__attribute__((annotate("Annotated"))) __attribute__((nonnull(1))) void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));
88+
89+
[[gnu::nonnull(1)]] [[gnu::always_inline]] void cxx11_attr(int*) ANNOTATE_ATTR;
90+
// CHECK: {{\[\[}}gnu::nonnull(1)]] {{\[\[}}gnu::always_inline]] void cxx11_attr(int *) __attribute__((annotate("Annotated")));

‎clang/test/Analysis/blocks.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void testBlockWithCaptureByReference() {
7878
// CHECK-NEXT: 1: 5
7979
// WARNINGS-NEXT: 2: [B1.1] (CXXConstructExpr, StructWithCopyConstructor)
8080
// ANALYZER-NEXT: 2: [B1.1] (CXXConstructExpr, [B1.3], StructWithCopyConstructor)
81-
// CHECK-NEXT: 3: StructWithCopyConstructor s(5) __attribute__((blocks("byref")));
81+
// CHECK-NEXT: 3: __attribute__((blocks("byref"))) StructWithCopyConstructor s(5);
8282
// CHECK-NEXT: 4: ^{ }
8383
// CHECK-NEXT: 5: (void)([B1.4]) (CStyleCastExpr, ToVoid, void)
8484
// CHECK-NEXT: Preds (1): B2

‎clang/test/Analysis/scopes-cfg-output.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1469,7 +1469,7 @@ void test_cleanup_functions2(int m) {
14691469
// CHECK: [B1]
14701470
// CHECK-NEXT: 1: CFGScopeBegin(f)
14711471
// CHECK-NEXT: 2: (CXXConstructExpr, [B1.3], F)
1472-
// CHECK-NEXT: 3: __attribute__((cleanup(cleanup_F))) F f;
1472+
// CHECK-NEXT: 3: F f __attribute__((cleanup(cleanup_F)));
14731473
// CHECK-NEXT: 4: CleanupFunction (cleanup_F)
14741474
// CHECK-NEXT: 5: [B1.3].~F() (Implicit destructor)
14751475
// CHECK-NEXT: 6: CFGScopeEnd(f)

‎clang/test/Sema/attr-print.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
// CHECK: int x __attribute__((aligned(4)));
44
int x __attribute__((aligned(4)));
55

6-
// FIXME: Print this at a valid location for a __declspec attr.
7-
// CHECK: int y __declspec(align(4));
6+
// CHECK: __declspec(align(4)) int y;
87
__declspec(align(4)) int y;
98

109
// CHECK: short arr[3] __attribute__((aligned));

‎clang/test/SemaCXX/attr-no-sanitize.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ int f3() __attribute__((no_sanitize("address")));
1616

1717
// DUMP-LABEL: FunctionDecl {{.*}} f4
1818
// DUMP: NoSanitizeAttr {{.*}} thread
19-
// PRINT: int f4() {{\[\[}}clang::no_sanitize("thread")]]
19+
// PRINT: {{\[\[}}clang::no_sanitize("thread")]] int f4()
2020
[[clang::no_sanitize("thread")]] int f4();
2121

2222
// DUMP-LABEL: FunctionDecl {{.*}} f4
2323
// DUMP: NoSanitizeAttr {{.*}} hwaddress
24-
// PRINT: int f4() {{\[\[}}clang::no_sanitize("hwaddress")]]
24+
// PRINT: {{\[\[}}clang::no_sanitize("hwaddress")]] int f4()
2525
[[clang::no_sanitize("hwaddress")]] int f4();
2626

2727
// DUMP-LABEL: FunctionDecl {{.*}} f5
@@ -36,5 +36,5 @@ int f6() __attribute__((no_sanitize("unknown"))); // expected-warning{{unknown s
3636

3737
// DUMP-LABEL: FunctionDecl {{.*}} f7
3838
// DUMP: NoSanitizeAttr {{.*}} memtag
39-
// PRINT: int f7() {{\[\[}}clang::no_sanitize("memtag")]]
39+
// PRINT: {{\[\[}}clang::no_sanitize("memtag")]] int f7()
4040
[[clang::no_sanitize("memtag")]] int f7();

‎clang/test/SemaCXX/attr-print.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
// CHECK: int x __attribute__((aligned(4)));
44
int x __attribute__((aligned(4)));
55

6-
// FIXME: Print this at a valid location for a __declspec attr.
7-
// CHECK: int y __declspec(align(4));
6+
// CHECK: __declspec(align(4)) int y;
87
__declspec(align(4)) int y;
98

109
// CHECK: int foo() __attribute__((const));

‎clang/test/SemaCXX/cxx11-attr-print.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
// CHECK: int x __attribute__((aligned(4)));
44
int x __attribute__((aligned(4)));
55

6-
// FIXME: Print this at a valid location for a __declspec attr.
7-
// CHECK: int y __declspec(align(4));
6+
// CHECK: __declspec(align(4)) int y;
87
__declspec(align(4)) int y;
98

109
// CHECK: int z {{\[}}[gnu::aligned(4)]];
@@ -25,10 +24,10 @@ int d [[deprecated("warning")]];
2524
// CHECK: __attribute__((deprecated("warning", "fixit")));
2625
int e __attribute__((deprecated("warning", "fixit")));
2726

28-
// CHECK: int cxx11_alignas alignas(4);
27+
// CHECK: alignas(4) int cxx11_alignas;
2928
alignas(4) int cxx11_alignas;
3029

31-
// CHECK: int c11_alignas _Alignas(int);
30+
// CHECK: _Alignas(int) int c11_alignas;
3231
_Alignas(int) int c11_alignas;
3332

3433
// CHECK: int foo() __attribute__((const));
@@ -66,8 +65,8 @@ void f8 (void *, const char *, ...) __attribute__ ((format (printf, 2, 3)));
6665
// CHECK: int m __attribute__((aligned(4
6766
// CHECK: int n alignas(4
6867
// CHECK: int p alignas(int
69-
// CHECK: static int f() __attribute__((pure))
70-
// CHECK: static int g() {{\[}}[gnu::pure]]
68+
// CHECK: __attribute__((pure)) static int f()
69+
// CHECK: {{\[}}[gnu::pure]] static int g()
7170
template <typename T> struct S {
7271
__attribute__((aligned(4))) int m;
7372
alignas(4) int n;
@@ -82,8 +81,8 @@ template <typename T> struct S {
8281

8382
// CHECK: int m __attribute__((aligned(4
8483
// CHECK: int n alignas(4
85-
// CHECK: static int f() __attribute__((pure))
86-
// CHECK: static int g() {{\[}}[gnu::pure]]
84+
// CHECK: __attribute__((pure)) static int f()
85+
// CHECK: {{\[}}[gnu::pure]] static int g()
8786
template struct S<int>;
8887

8988
// CHECK: using Small2 {{\[}}[gnu::mode(byte)]] = int;

‎clang/utils/TableGen/ClangAttrEmitter.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,18 +1591,18 @@ writePrettyPrintFunction(const Record &R,
15911591
std::string Variety = Spellings[I].variety();
15921592

15931593
if (Variety == "GNU") {
1594-
Prefix = " __attribute__((";
1594+
Prefix = "__attribute__((";
15951595
Suffix = "))";
15961596
} else if (Variety == "CXX11" || Variety == "C23") {
1597-
Prefix = " [[";
1597+
Prefix = "[[";
15981598
Suffix = "]]";
15991599
std::string Namespace = Spellings[I].nameSpace();
16001600
if (!Namespace.empty()) {
16011601
Spelling += Namespace;
16021602
Spelling += "::";
16031603
}
16041604
} else if (Variety == "Declspec") {
1605-
Prefix = " __declspec(";
1605+
Prefix = "__declspec(";
16061606
Suffix = ")";
16071607
} else if (Variety == "Microsoft") {
16081608
Prefix = "[";

0 commit comments

Comments
 (0)