[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev created
https://github.com/llvm/llvm-project/pull/87281
Commit https://github.com/llvm/llvm-project/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 https://github.com/llvm/llvm-project/issues/87151
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 1/5] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 2/5] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 3/5] Revert
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: @giulianobelinassi, I could not select you for a reviewer but please take a look at the pull request. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Vassil Vassilev (vgvassilev)
Changes
Commit https://github.com/llvm/llvm-project/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 https://github.com/llvm/llvm-project/issues/87151
---
Patch is 31.51 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/87281.diff
16 Files Affected:
- (modified) clang/include/clang/Basic/Attr.td (+1-13)
- (modified) clang/include/clang/Basic/CMakeLists.txt (-10)
- (modified) clang/lib/AST/DeclPrinter.cpp (+45-132)
- (removed) clang/test/AST/ast-print-attr-knr.c (-17)
- (modified) clang/test/AST/ast-print-method-decl.cpp (+1-1)
- (modified) clang/test/AST/ast-print-no-sanitize.cpp (+1-1)
- (modified) clang/test/Analysis/scopes-cfg-output.cpp (+1-1)
- (modified) clang/test/OpenMP/assumes_codegen.cpp (+9-9)
- (modified) clang/test/OpenMP/assumes_print.cpp (+1-1)
- (modified) clang/test/OpenMP/assumes_template_print.cpp (+7-7)
- (modified) clang/test/OpenMP/declare_simd_ast_print.cpp (+2-2)
- (modified) clang/test/SemaCXX/attr-no-sanitize.cpp (+3-3)
- (modified) clang/test/SemaCXX/cxx11-attr-print.cpp (+4-4)
- (modified) clang/utils/TableGen/ClangAttrEmitter.cpp (-31)
- (modified) clang/utils/TableGen/TableGen.cpp (-16)
- (modified) clang/utils/TableGen/TableGenBackends.h (-2)
``diff
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 3e03e55612645b..bb77f62405fbec 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -324,13 +324,10 @@ class Spelling {
}
class GNU : Spelling;
-class Declspec : Spelling {
- bit PrintOnLeft = 1;
-}
+class Declspec : Spelling;
class Microsoft : Spelling;
class CXX11
: Spelling {
- bit CanPrintOnLeft = 0;
string Namespace = namespace;
}
class C23
@@ -596,12 +593,6 @@ class AttrSubjectMatcherAggregateRule
{
def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule;
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 Spellings;
// The things to which an attribute can appertain
@@ -937,7 +928,6 @@ def AVRSignal : InheritableAttr,
TargetSpecificAttr {
}
def AsmLabel : InheritableAttr {
- let CanPrintOnLeft = 0;
let Spellings = [CustomKeyword<"asm">, CustomKeyword<"__asm__">];
let Args = [
// Label specifies the mangled name for the decl.
@@ -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.
@@ -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.
diff --git a/clang/include/clang/Basic/CMakeLists.txt
b/clang/include/clang/Basic/CMakeLists.txt
index 7d53c751c13ac4..2ef6ddc68f4bf3 100644
--- a/clang/include/clang/Basic/CMakeLists.txt
+++ b/clang/include/clang/Basic/CMakeLists.txt
@@ -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
diff --git a/clang/lib/AST/DeclPrinter.cpp
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 1/5] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 2/5] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 3/5] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp | 7
[clang] Rework the printing of attributes (PR #87281)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +241,43 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
-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;
+static DeclPrinter::AttrPosAsWritten getAttrPosAsWritten(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;
- return canPrintOnLeftSide(A->getKind());
-}
+ if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
+return DeclPrinter::AttrPosAsWritten::Left;
-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
+ return DeclPrinter::AttrPosAsWritten::Right;
}
-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 P /*=Unknown*/) {
erichkeane wrote:
A rename of 'P' to make it clear that `prettyPrintAttributes` is running in
"LHS", "RHS", or "Unknown" mode would be helpful.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -129,11 +118,13 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef Args,
const TemplateParameterList *Params);
-
-inline void prettyPrintAttributes(Decl *D) {
- prettyPrintAttributes(D, Out);
-}
-
+enum class AttrPosAsWritten {
+ Unknown = 0,
erichkeane wrote:
'Unknown' is likely to happen with 'implicit' attributes. Perhaps the
'CanPrintOnLeft' is worth keeping so that we can aggressively print those on
the left?
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +241,43 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
-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;
+static DeclPrinter::AttrPosAsWritten getAttrPosAsWritten(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;
- return canPrintOnLeftSide(A->getKind());
-}
+ if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
+return DeclPrinter::AttrPosAsWritten::Left;
-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
+ return DeclPrinter::AttrPosAsWritten::Right;
}
-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 P /*=Unknown*/) {
if (Policy.PolishForDeclaration)
return;
if (D->hasAttrs()) {
-AttrVec &Attrs = D->getAttrs();
+const AttrVec &Attrs = D->getAttrs();
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(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(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:
+if (P == AttrPosAsWritten::Unknown || P == getAttrPosAsWritten(A, D)) {
erichkeane wrote:
I'm not sure I get the logic of 'unknown' mode here, why are we printing ALL
attributes here?
Actually, when does that happen? It seems to me that when 'P' is unknown, we
should assert. However, when `getAttrPosAsWritten` is unknown, we should
probably print it 'on the left'?
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +241,43 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
-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;
+static DeclPrinter::AttrPosAsWritten getAttrPosAsWritten(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;
- return canPrintOnLeftSide(A->getKind());
-}
+ if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
erichkeane wrote:
I'm not particularly up-to-date on source-location comparisons, but I hope you
have a test to make sure that this works for an attribute defined in a macro,
but put on the RHS.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/erichkeane commented: Since this adds so much logic, we definitely need tests that handle when the attribute is defined in a macro, when there are multiples on each side, when there are multiples in macros/etc. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -129,11 +118,13 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef Args,
const TemplateParameterList *Params);
-
-inline void prettyPrintAttributes(Decl *D) {
- prettyPrintAttributes(D, Out);
-}
-
+enum class AttrPosAsWritten {
+ Unknown = 0,
vgvassilev wrote:
Unknown means that we have no preference and we want to keep the old behavior
of `prettyPrintAttributes`.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +241,43 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
-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;
+static DeclPrinter::AttrPosAsWritten getAttrPosAsWritten(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;
- return canPrintOnLeftSide(A->getKind());
-}
+ if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
+return DeclPrinter::AttrPosAsWritten::Left;
-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
+ return DeclPrinter::AttrPosAsWritten::Right;
}
-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 P /*=Unknown*/) {
if (Policy.PolishForDeclaration)
return;
if (D->hasAttrs()) {
-AttrVec &Attrs = D->getAttrs();
+const AttrVec &Attrs = D->getAttrs();
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(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(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:
+if (P == AttrPosAsWritten::Unknown || P == getAttrPosAsWritten(A, D)) {
vgvassilev wrote:
If the mode is "Unknown" that means we want the "regular/old" behavior where we
don't have a left/right preference. I realize that's probably not the best
terminology I use but let me know if you have better ideas.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
giulianobelinassi wrote: I am not sure how I feel about dropping the canPrintOnRight/printOnRight logic. We use it as a fallback when the SourceRange of a function is unreliable, otherwise we always copy and paste the code the user wrote. Regardless of that I will check for fallbacks on clang-extract once I get it to link with this branch. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -73,3 +73,15 @@ 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;
vgvassilev wrote:
Is that something this PR breaks? I am not sure I understand if I need to
change anything.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -73,3 +73,15 @@ 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;
AaronBallman wrote:
The PR changes behavior from one really broken state to a slightly less but
still broken state. Consider:
```
[[clang::annotate("do the")]] int [[clang::annotate_type("test")]] *
[[clang::annotate_type("again")]] i [[clang::annotate("and again!")]] = 0;
```
Currently we print this back as: `int *i [[clang::annotate_type(...)]]
[[clang::annotate_type(...)]] [[clang::annotate("do the")]]
[[clang::annotate("and again!")]] = 0;` which is all kinds of wrong. Now we
print this back as: `[[clang::annotate("do the")]] int *i
[[clang::annotate_type(...)]] [[clang::annotate_type(...)]]
[[clang::annotate("and again!")]] = 0;` which is in better shape but still gets
the type attributes wrong by moving the attribute off `int` and `*` and onto
the declaration of `i`. The new code won't compile (it tries to add type
attributes to a declaration, and the `...` instead of the original string
literal won't compile either).
Given that printing is best-effort, perhaps this is fine because it's
incremental improvement, but we're still not quite there in terms of accurate
printing either.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 1/9] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 2/9] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 3/9] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp | 7
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +238,47 @@ 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();
+const AttrVec &Attrs = D->getAttrs();
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(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(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);
+assert(APos != AttrPosAsWritten::Unknown && "Implicit attribute!");
vgvassilev wrote:
Comment added.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +238,47 @@ 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();
+const AttrVec &Attrs = D->getAttrs();
vgvassilev wrote:
I added an assert in the if-stmt. Is that what you meant?
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +238,50 @@ 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();
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(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(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 with no source
+// locations.
+assert(APos != AttrPosAsWritten::Unknown && "Implicit attribute!");
erichkeane wrote:
```suggestion
assert(APos != AttrPosAsWritten::Unknown && "Invalid source location
for attribute or decl.");
assert(APos !=AttrPosAsWritten::Default && "Default not a valid for an
attribute location");
```
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +238,50 @@ 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();
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(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(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 with no source
erichkeane wrote:
```suggestion
// Might trigger on programatically created attributes or declarations
with no source
```
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
@@ -250,87 +238,47 @@ 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();
+const AttrVec &Attrs = D->getAttrs();
erichkeane wrote:
I see that, it does what I want, yes.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 01/10] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 02/10] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 03/10] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 01/11] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 02/11] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 03/11] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 1/5] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 2/5] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 3/5] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp | 7
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/87281
>From 94f1c8653903cc3db55abd641c68a9dd11f05df3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:33 +
Subject: [PATCH 1/5] Revert "Remove extra switch from 0323938d"
This reverts commit 6c0b9e35761738726aded3b399db89eb0a86f82b.
---
clang/lib/AST/DeclPrinter.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index edbcdfe4d55bc9..1ab74c71d0fd49 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -277,6 +277,7 @@ static bool canPrintOnLeftSide(const Attr *A) {
}
static bool mustPrintOnLeftSide(attr::Kind kind) {
+ switch (kind) {
#ifdef CLANG_ATTR_LIST_PrintOnLeft
switch (kind) {
CLANG_ATTR_LIST_PrintOnLeft
>From 871f5b06e0da167c4cf50d336bdc38f0eb684b6b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:01:44 +
Subject: [PATCH 2/5] Revert "Fix warning in MSVC"
This reverts commit 0323938d3c7a1a48b60a7dea8ec7300e98b4a931.
---
clang/lib/AST/DeclPrinter.cpp | 20 +++-
clang/utils/TableGen/ClangAttrEmitter.cpp | 11 +--
2 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 1ab74c71d0fd49..c2e0edd5f1f12c 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -250,23 +250,13 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
return Out;
}
-// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideCanPrintList.inc"
-
-// For CLANG_ATTR_LIST_PrintOnLeft macro.
-#include "clang/Basic/AttrLeftSideMustPrintList.inc"
-
static bool canPrintOnLeftSide(attr::Kind kind) {
-#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
switch (kind) {
- CLANG_ATTR_LIST_CanPrintOnLeft
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool canPrintOnLeftSide(const Attr *A) {
@@ -278,16 +268,11 @@ static bool canPrintOnLeftSide(const Attr *A) {
static bool mustPrintOnLeftSide(attr::Kind kind) {
switch (kind) {
-#ifdef CLANG_ATTR_LIST_PrintOnLeft
- switch (kind) {
- CLANG_ATTR_LIST_PrintOnLeft
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
return true;
default:
return false;
}
-#else
- return false;
-#endif
}
static bool mustPrintOnLeftSide(const Attr *A) {
@@ -329,6 +314,7 @@ void DeclPrinter::prettyPrintAttributes(Decl *D,
llvm::raw_ostream &Out,
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
+
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index eb5c34d15693d7..985cc1aee579e9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3327,8 +3327,6 @@ void EmitClangAttrPrintList(const std::string &FieldName,
RecordKeeper &Records,
std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
std::vector PragmaAttrs;
- bool first = false;
-
for (auto *Attr : Attrs) {
if (!Attr->getValueAsBit("ASTNode"))
continue;
@@ -3336,15 +3334,8 @@ void EmitClangAttrPrintList(const std::string
&FieldName, RecordKeeper &Records,
if (!Attr->getValueAsBit(FieldName))
continue;
-if (!first) {
- first = true;
- OS << "#define CLANG_ATTR_LIST_" << FieldName;
-}
-
-OS << " \\\n case attr::" << Attr->getName() << ":";
+OS << "case attr::" << Attr->getName() << ":\n";
}
-
- OS << '\n';
}
// Emits the enumeration list for attributes.
>From e7e2e90da2a9644b0ca8a4c5c464c75a35f019a5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Mon, 1 Apr 2024 15:11:28 +
Subject: [PATCH 3/5] Revert "Fix ast print of variables with attributes"
This reverts commit 46f3ade5083b8bfce55c78a21086a487eaac6f99.
---
clang/include/clang/Basic/Attr.td| 14 +-
clang/include/clang/Basic/CMakeLists.txt | 10 --
clang/lib/AST/DeclPrinter.cpp| 137 +++
clang/test/AST/ast-print-attr-knr.c | 17 ---
clang/test/AST/ast-print-attr.c | 6 -
clang/test/AST/ast-print-method-decl.cpp | 2 +-
clang/test/AST/ast-print-pragmas.cpp | 4 +-
clang/test/Analysis/blocks.mm| 2 +-
clang/test/OpenMP/assumes_codegen.cpp| 22 +--
clang/test/OpenMP/assumes_print.cpp | 6 +-
clang/test/OpenMP/assumes_template_print.cpp | 20 +--
clang/test/OpenMP/declare_simd_ast_print.cpp | 4 +-
clang/test/Sema/attr-print.c | 3 +-
clang/test/SemaCXX/attr-print.cpp| 3 +-
clang/test/SemaCXX/cxx11-attr-print.cpp | 7
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: @erichkeane, thank you. What's the process of including this in the next release? https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
erichkeane wrote: > @erichkeane, thank you. What's the process of including this in the next > release? After CI is complete, you can click "Squash and Merge" below (if you cannot, let us know and someone can do it for you), and it'll be included in the 19.1 release this summer. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: > > @erichkeane, thank you. What's the process of including this in the next > > release? > > After CI is complete, you can click "Squash and Merge" below (if you cannot, > let us know and someone can do it for you), and it'll be included in the 19.1 > release this summer. I have commit access. I want this to be part of the 18.x releases as that breaks our downstream clients. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
erichkeane wrote: > > > @erichkeane, thank you. What's the process of including this in the next > > > release? > > > > > > After CI is complete, you can click "Squash and Merge" below (if you > > cannot, let us know and someone can do it for you), and it'll be included > > in the 19.1 release this summer. > > I have commit access. I want this to be part of the 18.x releases as that > breaks our downstream clients. Ah, hmm... I am not sure this qualifies for inclusion in the current release branch. Perhaps @AaronBallman can comment here. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: In https://github.com/llvm/llvm-project/issues/87151 is more context. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
https://github.com/vgvassilev closed https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
AaronBallman wrote: > > > > @erichkeane, thank you. What's the process of including this in the > > > > next release? > > > > > > > > > After CI is complete, you can click "Squash and Merge" below (if you > > > cannot, let us know and someone can do it for you), and it'll be included > > > in the 19.1 release this summer. > > > > > > I have commit access. I want this to be part of the 18.x releases as that > > breaks our downstream clients. > > Ah, hmm... I am not sure this qualifies for inclusion in the current release > branch. Perhaps @AaronBallman can comment here. I'm not particularly comfortable putting this into 18.x; we should only be pushing very safe fixes to regressions and this one doesn't really qualify. It's debatable whether it's a regression (it sort of is, sort of isn't), but the changes are also relatively involved. I'd feel more comfortable with this in 19.x so we have more time to find and fix remaining edge cases. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: Ok. Fair enough. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
giulianobelinassi wrote:
> > > > > @erichkeane, thank you. What's the process of including this in the
> > > > > next release?
> > > >
> > > >
> > > > After CI is complete, you can click "Squash and Merge" below (if you
> > > > cannot, let us know and someone can do it for you), and it'll be
> > > > included in the 19.1 release this summer.
> > >
> > >
> > > I have commit access. I want this to be part of the 18.x releases as that
> > > breaks our downstream clients.
> >
> >
> > Ah, hmm... I am not sure this qualifies for inclusion in the current
> > release branch. Perhaps @AaronBallman can comment here.
>
> I'm not particularly comfortable putting this into 18.x; we should only be
> pushing very safe fixes to regressions and this one doesn't really qualify.
> It's debatable whether it's a regression (it sort of is, sort of isn't), but
> the changes are also relatively involved. I'd feel more comfortable with this
> in 19.x so we have more time to find and fix remaining edge cases.
How about this single-line fix? It would fix the override regression for now.
```
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 80e607525a0a..a39288464040 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2511,6 +2511,7 @@ def Overloadable : Attr {
}
def Override : InheritableAttr {
+ let CanPrintOnLeft = 0;
let Spellings = [CustomKeyword<"override">];
let SemaHandler = 0;
// Omitted from docs, since this is language syntax, not an attribute, as far
```
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: Great idea! That’d make sense to me. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
giulianobelinassi wrote: We would like this fixed in 18.1 as well. We are expanding to support C++ and we will hit this bug at some point. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: Maybe you can open a PR against the branch? https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
AaronBallman wrote: And `final` as well as `override`? (This is why I'm not convinced we should be backporting anything -- the problem is with printing in general and will crop up in various places, so we're not really fixing a regression so much as playing whack-a-mole with a few cases.) https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
giulianobelinassi wrote: > Maybe you can open a PR against the branch? Sorry, but I can only do this tomorrow. Feel free to open the PR if you need it immediately. > And `final` as well as `override`? (This is why I'm not convinced we should > be backporting anything -- the problem is with printing in general and will > crop up in various places, so we're not really fixing a regression so much as > playing whack-a-mole with a few cases.) Why not? `override` and perhaps `final` output is broken so I'd say we fix this on LLVM-18. It is better than leave it broken. I don't think this will break many test cases once this bug got undetected. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
AaronBallman wrote: > > And `final` as well as `override`? (This is why I'm not convinced we should > > be backporting anything -- the problem is with printing in general and will > > crop up in various places, so we're not really fixing a regression so much > > as playing whack-a-mole with a few cases.) > > Why not? `override` and perhaps `final` output is broken so I'd say we fix > this on LLVM-18. It is better than leave it broken. I don't think this will > break many test cases once this bug got undetected. "broken" is a bit subjective in this case -- this isn't a user-facing feature and it's always been best-effort. I realize a few folks would like to elevate it to be something more than best-effort, but it's still unclear how that works in practice and whether it's worth the ongoing maintenance burdens in upstream. Fixing a few corner cases does improve things and the fix is trivial to verify for correctness, so that's why I'm not strongly opposed. I'm just not certain it meets the usual criteria for inclusion in a dot release. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote: A note from left field: I think this PR broke the IWYU test suite. We use `TemplateDecl::print` + some postprocessing to generate template forward-declarations. We're seeing this diff now: ```diff -template class FinalTemplate; +template class FinalTemplate; ``` So a spurious extra space. We would be grateful if this _didn't_ make the 18.x branches, because we've already released an 18.x-compatible version of IWYU, and it would be a shame if that release had a failing test suite when built against later 18.x Clang. I'll work around this on our latest mainline. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
erichkeane wrote: > A note from left field: I think this PR broke the IWYU test suite. We use > `TemplateDecl::print` + some postprocessing to generate template > forward-declarations. We're seeing this diff now: > > ```diff > -template class FinalTemplate; > +template class FinalTemplate; > ``` > > So a spurious extra space. > > We would be grateful if this _didn't_ make the 18.x branches, because we've > already released an 18.x-compatible version of IWYU, and it would be a shame > if that release had a failing test suite when built against later 18.x Clang. > > I'll work around this on our latest mainline. Ooof, I think that DOES decide it for me. @vgvassilev : can you look into that extra space to see if you can track it down? https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote: @erichkeane Thanks! Let me just see if I can bisect it to this commit; I don't have any evidence yet, this PR was just the only significant change to `DeclPrinter.cpp` in the past few days. I need a little while to rebuild my local Clang tree with and without the patch. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote:
I can confirm that the double space comes from this PR;
```diff
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp
b/clang/unittests/AST/DeclPrinterTest.cpp
index c24e442621c9..c2d02e74a62c 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -1555,3 +1555,11 @@ TEST(DeclPrinter, VarDeclWithInitializer) {
PrintedDeclCXX17Matches("void foo() {int arr[42]; for(int a : arr);}",
namedDecl(hasName("a")).bind("id"), "int a"));
}
+
+TEST(DeclPrinter, TestTemplateFinal) {
+ ASSERT_TRUE(PrintedDeclCXX11Matches(
+ "template\n"
+ "class FinalTemplate final {};",
+ classTemplateDecl(hasName("FinalTemplate")).bind("id"),
+ "template class final FinalTemplate final {}"));
+}
```
fails with:
```
.../llvm-project/clang/unittests/AST/DeclPrinterTest.cpp:1560: [...]
Expected "template class final FinalTemplate final {}"
got "template class final FinalTemplate final {}")
```
(edited so it's easier to see the diff).
It passes after I revert the `Rework attributes` commits:
* 9391ff8c86007562d40c240ea082b7c0cbf35947
* 62e92573d28d62ab7e6438ac34d513b07c51ce09
* a30662fc2acdd73ca1a9217716299a4676999fb4
Again, from my point of view, this "bug" is fine on mainline, we can work
around it. But it would be nice if this patch was not backported to 18 as it
breaks our corresponding release.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote: (obtw, the double `final` is unrelated, it's tracked here: https://github.com/llvm/llvm-project/issues/56517) https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote:
> I can confirm that the double space comes from this PR;
>
> ```diff
> diff --git a/clang/unittests/AST/DeclPrinterTest.cpp
> b/clang/unittests/AST/DeclPrinterTest.cpp
> index c24e442621c9..c2d02e74a62c 100644
> --- a/clang/unittests/AST/DeclPrinterTest.cpp
> +++ b/clang/unittests/AST/DeclPrinterTest.cpp
> @@ -1555,3 +1555,11 @@ TEST(DeclPrinter, VarDeclWithInitializer) {
>PrintedDeclCXX17Matches("void foo() {int arr[42]; for(int a : arr);}",
>namedDecl(hasName("a")).bind("id"), "int a"));
> }
> +
> +TEST(DeclPrinter, TestTemplateFinal) {
> + ASSERT_TRUE(PrintedDeclCXX11Matches(
> + "template\n"
> + "class FinalTemplate final {};",
> + classTemplateDecl(hasName("FinalTemplate")).bind("id"),
> + "template class final FinalTemplate final {}"));
> +}
> ```
>
> fails with:
>
> ```
> .../llvm-project/clang/unittests/AST/DeclPrinterTest.cpp:1560: [...]
> Expected "template class final FinalTemplate final {}"
> got "template class final FinalTemplate final {}")
> ```
>
> (edited so it's easier to see the diff).
>
> It passes after I revert the `Rework attributes` commits:
>
> *
> [9391ff8](https://github.com/llvm/llvm-project/commit/9391ff8c86007562d40c240ea082b7c0cbf35947)
>
> *
> [62e9257](https://github.com/llvm/llvm-project/commit/62e92573d28d62ab7e6438ac34d513b07c51ce09)
>
> *
> [a30662f](https://github.com/llvm/llvm-project/commit/a30662fc2acdd73ca1a9217716299a4676999fb4)
>
>
> Again, from my point of view, this "bug" is fine on mainline, we can work
> around it. But it would be nice if this patch was not backported to 18 as it
> breaks our corresponding release.
@kimgr, than you for the report. I can reproduce the double space. However, do
you expect that test of yours to be valid C++? As written it seems not.
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote:
@vgvassilev I did expect the input to be valid, yes:
```
template
class FinalTemplate final {};
```
Is it not?
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote:
> @vgvassilev I did expect the input to be valid, yes:
>
> ```
> template
> class FinalTemplate final {};
> ```
>
> Is it not?
The snippet as visualized in github seems to have one too many `final`s:
`template class final FinalTemplate final {}`
https://github.com/llvm/llvm-project/pull/87281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
kimgr wrote: Ah, that's the expected output -- I can't do anything about that :). See https://github.com/llvm/llvm-project/issues/56517. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Rework the printing of attributes (PR #87281)
vgvassilev wrote: > Ah, that's the expected output -- I can't do anything about that :). See > #56517. I believe this should fix it: https://github.com/llvm/llvm-project/pull/88600 Can you test? https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
