[clang] Rework the printing of attributes (PR #87281)

2024-04-12 Thread Vassil Vassilev via cfe-commits

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


[clang] Rework the printing of attributes (PR #87281)

2024-04-12 Thread Kim Gräsman via cfe-commits

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)

2024-04-12 Thread Vassil Vassilev via cfe-commits

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)

2024-04-12 Thread Kim Gräsman via cfe-commits

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)

2024-04-12 Thread Vassil Vassilev via cfe-commits

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)

2024-04-11 Thread Kim Gräsman via cfe-commits

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)

2024-04-11 Thread Kim Gräsman via cfe-commits

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)

2024-04-11 Thread Kim Gräsman via cfe-commits

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)

2024-04-11 Thread Erich Keane via cfe-commits

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)

2024-04-11 Thread Kim Gräsman via cfe-commits

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)

2024-04-09 Thread Aaron Ballman via cfe-commits

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)

2024-04-09 Thread Giuliano Belinassi via cfe-commits

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)

2024-04-09 Thread Aaron Ballman via cfe-commits

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)

2024-04-09 Thread Vassil Vassilev via cfe-commits

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)

2024-04-09 Thread Giuliano Belinassi via cfe-commits

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)

2024-04-09 Thread Vassil Vassilev via cfe-commits

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)

2024-04-09 Thread Giuliano Belinassi via cfe-commits

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)

2024-04-09 Thread Vassil Vassilev via cfe-commits

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)

2024-04-09 Thread Aaron Ballman via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Erich Keane via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Erich Keane via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-08 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-08 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-08 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-08 Thread Erich Keane via cfe-commits

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)

2024-04-08 Thread Erich Keane via cfe-commits

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)

2024-04-08 Thread Vassil Vassilev via cfe-commits


@@ -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)

2024-04-08 Thread Vassil Vassilev via cfe-commits


@@ -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)

2024-04-08 Thread Vassil Vassilev via cfe-commits

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)

2024-04-05 Thread Aaron Ballman via cfe-commits


@@ -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)

2024-04-05 Thread Vassil Vassilev via cfe-commits


@@ -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)

2024-04-02 Thread Giuliano Belinassi via cfe-commits

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)

2024-04-02 Thread Vassil Vassilev via cfe-commits


@@ -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)

2024-04-02 Thread Vassil Vassilev via cfe-commits


@@ -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)

2024-04-02 Thread Erich Keane via cfe-commits

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)

2024-04-02 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-02 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-02 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-02 Thread Erich Keane via cfe-commits


@@ -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)

2024-04-02 Thread Erich Keane via cfe-commits

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)

2024-04-01 Thread Vassil Vassilev via cfe-commits

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)

2024-04-01 Thread via cfe-commits

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)

2024-04-01 Thread Vassil Vassilev via cfe-commits

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)

2024-04-01 Thread Vassil Vassilev via cfe-commits

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