Skip to content

Rework the printing of attributes #87281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 9, 2024
Merged

Rework the printing of attributes #87281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
94f1c86
Revert "Remove extra switch from 0323938d"
vgvassilev Apr 1, 2024
871f5b0
Revert "Fix warning in MSVC"
vgvassilev Apr 1, 2024
e7e2e90
Revert "Fix ast print of variables with attributes"
vgvassilev Apr 1, 2024
a72cf87
Rework the printing of attributes.
vgvassilev Apr 1, 2024
fd21bb2
OpenMP attributes now respect the pragma location
vgvassilev Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,13 +324,10 @@ class Spelling<string name, string variety, int version = 1> {
}

class GNU<string name> : Spelling<name, "GNU">;
class Declspec<string name> : Spelling<name, "Declspec"> {
bit PrintOnLeft = 1;
}
class Declspec<string name> : Spelling<name, "Declspec">;
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 @@ -596,12 +593,6 @@ 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 @@ -937,7 +928,6 @@ 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 @@ -1534,7 +1524,6 @@ 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 @@ -3149,7 +3138,6 @@ 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: 0 additions & 10 deletions clang/include/clang/Basic/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,6 @@ 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
184 changes: 52 additions & 132 deletions clang/lib/AST/DeclPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceManager.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;

Expand Down Expand Up @@ -49,18 +50,6 @@ 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 @@ -129,11 +118,10 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef<TemplateArgumentLoc> Args,
const TemplateParameterList *Params);

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

enum class AttrPosAsWritten { Unknown = 0, Default, Left, Right };
void
prettyPrintAttributes(const Decl *D,
AttrPosAsWritten Pos = AttrPosAsWritten::Default);
void prettyPrintPragmas(Decl *D);
void printDeclType(QualType T, StringRef DeclName, bool Pack = false);
};
Expand Down Expand Up @@ -250,87 +238,53 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}

// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
#include "clang/Basic/AttrLeftSideCanPrintList.inc"
static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A,
const Decl *D) {
SourceLocation ALoc = A->getLoc();
SourceLocation DLoc = D->getLocation();
const ASTContext &C = D->getASTContext();
if (ALoc.isInvalid() || DLoc.isInvalid())
return DeclPrinter::AttrPosAsWritten::Unknown;

// For CLANG_ATTR_LIST_PrintOnLeft macro.
#include "clang/Basic/AttrLeftSideMustPrintList.inc"
if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
return DeclPrinter::AttrPosAsWritten::Left;

static bool canPrintOnLeftSide(attr::Kind kind) {
#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_CanPrintOnLeft
return true;
default:
return false;
}
#else
return false;
#endif
}

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

return canPrintOnLeftSide(A->getKind());
return DeclPrinter::AttrPosAsWritten::Right;
}

static bool mustPrintOnLeftSide(attr::Kind kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
return true;
default:
return false;
}
#else
return false;
#endif
}

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) {
void DeclPrinter::prettyPrintAttributes(const Decl *D,
AttrPosAsWritten Pos /*=Default*/) {
if (Policy.PolishForDeclaration)
return;

if (D->hasAttrs()) {
AttrVec &Attrs = D->getAttrs();
assert(Pos != AttrPosAsWritten::Unknown && "Use Default");
const AttrVec &Attrs = D->getAttrs();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd like an assert here to make sure Pos != Unknown (or at the beginning of hte function).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an assert in the if-stmt. Is that what you meant?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that, it does what I want, yes.

for (auto *A : Attrs) {
if (A->isInherited() || A->isImplicit())
continue;

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;
switch (A->getKind()) {
#define ATTR(X)
#define PRAGMA_SPELLING_ATTR(X) case attr::X:
#include "clang/Basic/AttrList.inc"
break;
default:
AttrPosAsWritten APos = getPosAsWritten(A, D);
// Might trigger on programatically created attributes or declarations
// with no source locations.
assert(APos != AttrPosAsWritten::Unknown &&
"Invalid source location for attribute or decl.");
assert(APos != AttrPosAsWritten::Default &&
"Default not a valid for an attribute location");
if (Pos == AttrPosAsWritten::Default || Pos == APos) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case we are printing Left and Right, where do attributes we can't figure out based on location get printed? That is, where do Unknown location attributes get printed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have not specified Left or Right it prints them in their default position which seems predominantly on the right. In theory, we can rework all calls to prettyPrintAttributes and remove the need of Default because we will have:

printing_logic;
printAttributes(left);
more_printing_logic;
printAttributes(right);

That's doable but seemed outside of the scope of this PR since I am hoping to get it merged asap...

if (Pos != AttrPosAsWritten::Left)
Out << ' ';
A->printPretty(Out, Policy);
if (Pos == AttrPosAsWritten::Left)
Out << ' ';
}
break;
}
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
}
}
}
Expand Down Expand Up @@ -691,8 +645,10 @@ static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,

void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
if (!D->getDescribedFunctionTemplate() &&
!D->isFunctionTemplateSpecialization())
!D->isFunctionTemplateSpecialization()) {
prettyPrintPragmas(D);
prettyPrintAttributes(D, AttrPosAsWritten::Left);
}

if (D->isFunctionTemplateSpecialization())
Out << "template<> ";
Expand All @@ -702,22 +658,6 @@ 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 @@ -883,7 +823,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
Ty.print(Out, Policy, Proto);
}

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

if (D->isPureVirtual())
Out << " = 0";
Expand Down Expand Up @@ -976,27 +916,12 @@ void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
void DeclPrinter::VisitVarDecl(VarDecl *D) {
prettyPrintPragmas(D);

prettyPrintAttributes(D, AttrPosAsWritten::Left);

if (const auto *Param = dyn_cast<ParmVarDecl>(D);
Param && Param->isExplicitObjectParameter())
Out << "this ";

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 @@ -1029,21 +954,16 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
}
}

StringRef Name;

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

if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
!Policy.SuppressUnwrittenScope)
MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
printDeclType(T, Name);

// Print the attributes that should be placed right before the end of the
// decl.
prettyPrintAttributes(D, Out, AttrPrintLoc::Right);
printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
D->getIdentifier())
? D->getIdentifier()->deuglifiedName()
: D->getName());

prettyPrintAttributes(D, AttrPosAsWritten::Right);

Expr *Init = D->getInit();
if (!Policy.SuppressInitializers && Init) {
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/AST/StmtPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,11 @@ void StmtPrinter::VisitLabelStmt(LabelStmt *Node) {
}

void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
for (const auto *Attr : Node->getAttrs()) {
llvm::ArrayRef<const Attr *> Attrs = Node->getAttrs();
for (const auto *Attr : Attrs) {
Attr->printPretty(OS, Policy);
if (Attr != Attrs.back())
OS << ' ';
}

PrintStmt(Node->getSubStmt(), 0);
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-print-method-decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct DefMethodsWithoutBody {
// CHECK-NEXT: DefMethodsWithoutBody() = default;
~DefMethodsWithoutBody() = default;

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

// CHECK-NEXT: };
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-print-no-sanitize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ void should_not_crash_1() __attribute__((no_sanitize_memory));
[[clang::no_sanitize_memory]] void should_not_crash_2();

// CHECK: void should_not_crash_1() __attribute__((no_sanitize("memory")));
// CHECK: void should_not_crash_2() {{\[\[}}clang::no_sanitize("memory"){{\]\]}};
// CHECK: {{\[\[}}clang::no_sanitize("memory"){{\]\]}} void should_not_crash_2();
15 changes: 15 additions & 0 deletions clang/test/AST/attr-print-emit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,18 @@ class C {
// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
};

#define ANNOTATE_ATTR __attribute__((annotate("Annotated")))
ANNOTATE_ATTR int annotated_attr ANNOTATE_ATTR = 0;
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr __attribute__((annotate("Annotated"))) = 0;

// FIXME: We do not print the attribute as written after the type specifier.
int ANNOTATE_ATTR annotated_attr_fixme = 0;
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr_fixme = 0;

#define NONNULL_ATTR __attribute__((nonnull(1)))
ANNOTATE_ATTR NONNULL_ATTR void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));
// CHECK:__attribute__((annotate("Annotated"))) __attribute__((nonnull(1))) void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));

[[gnu::nonnull(1)]] [[gnu::always_inline]] void cxx11_attr(int*) ANNOTATE_ATTR;
// CHECK: {{\[\[}}gnu::nonnull(1)]] {{\[\[}}gnu::always_inline]] void cxx11_attr(int *) __attribute__((annotate("Annotated")));
2 changes: 1 addition & 1 deletion clang/test/Analysis/scopes-cfg-output.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ void test_cleanup_functions2(int m) {
// CHECK: [B1]
// CHECK-NEXT: 1: CFGScopeBegin(f)
// CHECK-NEXT: 2: (CXXConstructExpr, [B1.3], F)
// CHECK-NEXT: 3: __attribute__((cleanup(cleanup_F))) F f;
// CHECK-NEXT: 3: F f __attribute__((cleanup(cleanup_F)));
// CHECK-NEXT: 4: CleanupFunction (cleanup_F)
// CHECK-NEXT: 5: [B1.3].~F() (Implicit destructor)
// CHECK-NEXT: 6: CFGScopeEnd(f)
Expand Down
Loading