Skip to content

Commit

Permalink
Fix ast print of variables with attributes
Browse files Browse the repository at this point in the history
Previously clang AST prints the following declaration:

int fun_var_unused() {

  int x __attribute__((unused)) = 0;
  return x;
}

and

int __declspec(thread) x = 0;

as:

int fun_var_unused() {

  int x = 0 __attribute__((unused));
  return x;
}

and

int x = __declspec(thread) 0;

which is rejected by C/C++ parser. This patch modifies the logic to
print old C attributes for variables as:

int __attribute__((unused)) x = 0;
and the __declspec case as:

int __declspec(thread) x = 0;
Fixes: #59973

Previous version: D141714.

Differential Revision:https://reviews.llvm.org/D141714
  • Loading branch information
giulianobelinassi authored and erichkeane committed Sep 7, 2023
1 parent 04224d1 commit 46f3ade
Show file tree
Hide file tree
Showing 18 changed files with 243 additions and 55 deletions.
14 changes: 13 additions & 1 deletion clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,13 @@ class Spelling<string name, string variety, int version = 1> {
}

class GNU<string name> : Spelling<name, "GNU">;
class Declspec<string name> : Spelling<name, "Declspec">;
class Declspec<string name> : Spelling<name, "Declspec"> {
bit PrintOnLeft = 1;
}
class Microsoft<string name> : Spelling<name, "Microsoft">;
class CXX11<string namespace, string name, int version = 1>
: Spelling<name, "CXX11", version> {
bit CanPrintOnLeft = 0;
string Namespace = namespace;
}
class C23<string namespace, string name, int version = 1>
Expand Down Expand Up @@ -559,6 +562,12 @@ class AttrSubjectMatcherAggregateRule<AttrSubject subject> {
def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule<Named>;

class Attr {
// Specifies that when printed, this attribute is meaningful on the
// 'left side' of the declaration.
bit CanPrintOnLeft = 1;
// Specifies that when printed, this attribute is required to be printed on
// the 'left side' of the declaration.
bit PrintOnLeft = 0;
// The various ways in which an attribute can be spelled in source
list<Spelling> Spellings;
// The things to which an attribute can appertain
Expand Down Expand Up @@ -892,6 +901,7 @@ def AVRSignal : InheritableAttr, TargetSpecificAttr<TargetAVR> {
}

def AsmLabel : InheritableAttr {
let CanPrintOnLeft = 0;
let Spellings = [CustomKeyword<"asm">, CustomKeyword<"__asm__">];
let Args = [
// Label specifies the mangled name for the decl.
Expand Down Expand Up @@ -1434,6 +1444,7 @@ def AllocSize : InheritableAttr {
}

def EnableIf : InheritableAttr {
let CanPrintOnLeft = 0;
// Does not have a [[]] spelling because this attribute requires the ability
// to parse function arguments but the attribute is not written in the type
// position.
Expand Down Expand Up @@ -2920,6 +2931,7 @@ def Unavailable : InheritableAttr {
}

def DiagnoseIf : InheritableAttr {
let CanPrintOnLeft = 0;
// Does not have a [[]] spelling because this attribute requires the ability
// to parse function arguments but the attribute is not written in the type
// position.
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/Basic/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ clang_tablegen(AttrList.inc -gen-clang-attr-list
SOURCE Attr.td
TARGET ClangAttrList)

clang_tablegen(AttrLeftSideCanPrintList.inc -gen-clang-attr-can-print-left-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
TARGET ClangAttrCanPrintLeftList)

clang_tablegen(AttrLeftSideMustPrintList.inc -gen-clang-attr-must-print-left-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
TARGET ClangAttrMustPrintLeftList)

clang_tablegen(AttrSubMatchRulesList.inc -gen-clang-attr-subject-match-rule-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
Expand Down
139 changes: 123 additions & 16 deletions clang/lib/AST/DeclPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ namespace {

void PrintObjCTypeParams(ObjCTypeParamList *Params);

enum class AttrPrintLoc {
None = 0,
Left = 1,
Right = 2,
Any = Left | Right,

LLVM_MARK_AS_BITMASK_ENUM(/*DefaultValue=*/Any)
};

void prettyPrintAttributes(Decl *D, raw_ostream &out,
AttrPrintLoc loc = AttrPrintLoc::Any);

public:
DeclPrinter(raw_ostream &Out, const PrintingPolicy &Policy,
const ASTContext &Context, unsigned Indentation = 0,
Expand Down Expand Up @@ -117,7 +129,11 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef<TemplateArgumentLoc> Args,
const TemplateParameterList *Params);
void prettyPrintAttributes(Decl *D);

inline void prettyPrintAttributes(Decl *D) {
prettyPrintAttributes(D, Out);
}

void prettyPrintPragmas(Decl *D);
void printDeclType(QualType T, StringRef DeclName, bool Pack = false);
};
Expand Down Expand Up @@ -234,7 +250,40 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}

void DeclPrinter::prettyPrintAttributes(Decl *D) {
static bool canPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
}

static bool canPrintOnLeftSide(const Attr *A) {
if (A->isStandardAttributeSyntax())
return false;

return canPrintOnLeftSide(A->getKind());
}

static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
}

static bool mustPrintOnLeftSide(const Attr *A) {
if (A->isDeclspecAttribute())
return true;

return mustPrintOnLeftSide(A->getKind());
}

void DeclPrinter::prettyPrintAttributes(Decl *D, llvm::raw_ostream &Out,
AttrPrintLoc Loc) {
if (Policy.PolishForDeclaration)
return;

Expand All @@ -243,15 +292,32 @@ void DeclPrinter::prettyPrintAttributes(Decl *D) {
for (auto *A : Attrs) {
if (A->isInherited() || A->isImplicit())
continue;
switch (A->getKind()) {
#define ATTR(X)
#define PRAGMA_SPELLING_ATTR(X) case attr::X:
#include "clang/Basic/AttrList.inc"
break;
default:
A->printPretty(Out, Policy);
break;

AttrPrintLoc AttrLoc = AttrPrintLoc::Right;
if (mustPrintOnLeftSide(A)) {
// If we must always print on left side (e.g. declspec), then mark as
// so.
AttrLoc = AttrPrintLoc::Left;
} else if (canPrintOnLeftSide(A)) {
// For functions with body defined we print the attributes on the left
// side so that GCC accept our dumps as well.
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
FD && FD->isThisDeclarationADefinition())
// In case Decl is a function with a body, then attrs should be print
// on the left side.
AttrLoc = AttrPrintLoc::Left;

// In case it is a variable declaration with a ctor, then allow
// printing on the left side for readbility.
else if (const VarDecl *VD = dyn_cast<VarDecl>(D);
VD && VD->getInit() &&
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}

// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
}
}
}
Expand Down Expand Up @@ -613,6 +679,22 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
printTemplateParameters(D->getTemplateParameterList(I));
}

std::string LeftsideAttrs;
llvm::raw_string_ostream LSAS(LeftsideAttrs);

prettyPrintAttributes(D, LSAS, AttrPrintLoc::Left);

// prettyPrintAttributes print a space on left side of the attribute.
if (LeftsideAttrs[0] == ' ') {
// Skip the space prettyPrintAttributes generated.
LeftsideAttrs.erase(0, LeftsideAttrs.find_first_not_of(' '));

// Add a single space between the attribute and the Decl name.
LSAS << ' ';
}

Out << LeftsideAttrs;

CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
CXXConversionDecl *ConversionDecl = dyn_cast<CXXConversionDecl>(D);
CXXDeductionGuideDecl *GuideDecl = dyn_cast<CXXDeductionGuideDecl>(D);
Expand Down Expand Up @@ -774,7 +856,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
Ty.print(Out, Policy, Proto);
}

prettyPrintAttributes(D);
prettyPrintAttributes(D, Out, AttrPrintLoc::Right);

if (D->isPure())
Out << " = 0";
Expand Down Expand Up @@ -867,6 +949,23 @@ void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
void DeclPrinter::VisitVarDecl(VarDecl *D) {
prettyPrintPragmas(D);

std::string LeftSide;
llvm::raw_string_ostream LeftSideStream(LeftSide);

// Print attributes that should be placed on the left, such as __declspec.
prettyPrintAttributes(D, LeftSideStream, AttrPrintLoc::Left);

// prettyPrintAttributes print a space on left side of the attribute.
if (LeftSide[0] == ' ') {
// Skip the space prettyPrintAttributes generated.
LeftSide.erase(0, LeftSide.find_first_not_of(' '));

// Add a single space between the attribute and the Decl name.
LeftSideStream << ' ';
}

Out << LeftSide;

QualType T = D->getTypeSourceInfo()
? D->getTypeSourceInfo()->getType()
: D->getASTContext().getUnqualifiedObjCPointerType(D->getType());
Expand Down Expand Up @@ -899,10 +998,19 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
}
}

printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
D->getIdentifier())
? D->getIdentifier()->deuglifiedName()
: D->getName());
StringRef Name;

Name = (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
D->getIdentifier())
? D->getIdentifier()->deuglifiedName()
: D->getName();

printDeclType(T, Name);

// Print the attributes that should be placed right before the end of the
// decl.
prettyPrintAttributes(D, Out, AttrPrintLoc::Right);

Expr *Init = D->getInit();
if (!Policy.SuppressInitializers && Init) {
bool ImplicitInit = false;
Expand Down Expand Up @@ -931,7 +1039,6 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
Out << ")";
}
}
prettyPrintAttributes(D);
}

void DeclPrinter::VisitParmVarDecl(ParmVarDecl *D) {
Expand Down
17 changes: 17 additions & 0 deletions clang/test/AST/ast-print-attr-knr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// This file contain tests for attribute arguments on K&R functions.

// RUN: %clang_cc1 -ast-print -x c -std=c89 -fms-extensions %s -o - | FileCheck %s

// CHECK: int knr(i)
// CHECK-NEXT: int i __attribute__((unused));
// CHECK-NEXT: {
// CHECK-NEXT: return 0;
// CHECK-NEXT: }
int knr(i) int i __attribute__((unused)); { return 0; }

// CHECK: __attribute__((unused)) int knr2(i)
// CHECK-NEXT: int i;
// CHECK-NEXT: {
// CHECK-NEXT: return 0;
// CHECK-NEXT: }
__attribute__((unused)) int knr2(i) int i; { return 0; }
6 changes: 6 additions & 0 deletions clang/test/AST/ast-print-attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,9 @@ int *fun_returns() __attribute__((ownership_returns(fun_returns)));

// CHECK: void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));
void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));

// CHECK: int fun_var_unused() {
// CHECK-NEXT: int x __attribute__((unused)) = 0;
// CHECK-NEXT: return x;
// CHECK-NEXT: }
int fun_var_unused() { int x __attribute__((unused)) = 0; return x; }
2 changes: 1 addition & 1 deletion clang/test/AST/ast-print-method-decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct DefMethodsWithoutBody {
// CHECK-NEXT: DefMethodsWithoutBody() = default;
~DefMethodsWithoutBody() = default;

// CHECK-NEXT: void m1() __attribute__((alias("X")));
// CHECK-NEXT: __attribute__((alias("X"))) void m1();
void m1() __attribute__((alias("X")));

// CHECK-NEXT: };
Expand Down
4 changes: 2 additions & 2 deletions clang/test/AST/ast-print-pragmas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ void test_templates(int *List, int Length) {
#ifdef MS_EXT
#pragma init_seg(compiler)
// MS-EXT: #pragma init_seg (.CRT$XCC){{$}}
// MS-EXT-NEXT: int x = 3 __declspec(thread);
int __declspec(thread) x = 3;
// MS-EXT-NEXT: __declspec(thread) int x = 3;
__declspec(thread) int x = 3;
#endif //MS_EXT

2 changes: 1 addition & 1 deletion clang/test/Analysis/blocks.mm
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void testBlockWithCaptureByReference() {
// CHECK-NEXT: 1: 5
// WARNINGS-NEXT: 2: [B1.1] (CXXConstructExpr, StructWithCopyConstructor)
// ANALYZER-NEXT: 2: [B1.1] (CXXConstructExpr, [B1.3], StructWithCopyConstructor)
// CHECK-NEXT: 3: StructWithCopyConstructor s(5) __attribute__((blocks("byref")));
// CHECK-NEXT: 3: __attribute__((blocks("byref"))) StructWithCopyConstructor s(5);
// CHECK-NEXT: 4: ^{ }
// CHECK-NEXT: 5: (void)([B1.4]) (CStyleCastExpr, ToVoid, void)
// CHECK-NEXT: Preds (1): B2
Expand Down
22 changes: 11 additions & 11 deletions clang/test/OpenMP/assumes_codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,41 +67,41 @@ int lambda_outer() {
}
#pragma omp end assumes

// AST: void foo() __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST: __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void foo() {
// AST-NEXT: }
// AST-NEXT: class BAR {
// AST-NEXT: public:
// AST-NEXT: BAR() __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAR() {
// AST-NEXT: }
// AST-NEXT: void bar1() __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void bar1() {
// AST-NEXT: }
// AST-NEXT: static void bar2() __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) static void bar2() {
// AST-NEXT: }
// AST-NEXT: };
// AST-NEXT: void bar() __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void bar() {
// AST-NEXT: BAR b;
// AST-NEXT: }
// AST-NEXT: void baz() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
// AST-NEXT: template <typename T> class BAZ {
// AST-NEXT: public:
// AST-NEXT: BAZ<T>() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAZ<T>() {
// AST-NEXT: }
// AST-NEXT: void baz1() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void baz1() {
// AST-NEXT: }
// AST-NEXT: static void baz2() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) static void baz2() {
// AST-NEXT: }
// AST-NEXT: };
// AST-NEXT: template<> class BAZ<float> {
// AST-NEXT: public:
// AST-NEXT: BAZ() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAZ() {
// AST-NEXT: }
// AST-NEXT: void baz1() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
// AST-NEXT: static void baz2() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
// AST-NEXT: };
// AST-NEXT: void baz() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void baz() {
// AST-NEXT: BAZ<float> b;
// AST-NEXT: }
// AST-NEXT: int lambda_outer() __attribute__((assume("ompx_lambda_assumption"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) {
// AST-NEXT: __attribute__((assume("ompx_lambda_assumption"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) int lambda_outer() {
// AST-NEXT: auto lambda_inner = []() {
// AST-NEXT: return 42;
// AST-NEXT: };
Expand Down
6 changes: 3 additions & 3 deletions clang/test/OpenMP/assumes_print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ void baz() {
}
#pragma omp end assumes

// CHECK: void foo() __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp")))
// CHECK: void bar() __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp")))
// CHECK: void baz() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp")))
// CHECK: __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void foo()
// CHECK: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void bar()
// CHECK: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void baz()

#endif

0 comments on commit 46f3ade

Please sign in to comment.