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
___
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
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 {}`
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
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
> +++
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
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
+++
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.
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;
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
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
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
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.)
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
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
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
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
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
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
> > >
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
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
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
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.
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.
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://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
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
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
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
@@ -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,
+
@@ -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,
+
@@ -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,
+
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
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
@@ -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,
+
@@ -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,
+
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
@@ -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
@@ -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
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
@@ -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
@@ -129,11 +118,13 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef Args,
const TemplateParameterList *Params);
-
-inline void prettyPrintAttributes(Decl *D)
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
@@ -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
@@ -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
@@ -129,11 +118,13 @@ namespace {
const TemplateParameterList *Params);
void printTemplateArguments(llvm::ArrayRef Args,
const TemplateParameterList *Params);
-
-inline void prettyPrintAttributes(Decl *D)
@@ -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
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
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
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
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://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
52 matches
Mail list logo
