kimgr wrote:
@vgvassilev Thank you!
https://github.com/llvm/llvm-project/pull/88600
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vgvassilev closed
https://github.com/llvm/llvm-project/pull/88600
___
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/88600
>From 9b2bb9068cbefcfffd0931fbbee46b1a0f536a4f Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH 1/2] Fix the double space and double attribute printing of
@@ -275,13 +278,15 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D,
if (Pos != AttrPosAsWritten::Left)
Out << ' ';
A->printPretty(Out, Policy);
+ hasPrinted = true;
if (Pos == AttrPosAsWritten::Left)
Out
@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten
getPosAsWritten(const Attr *A,
return DeclPrinter::AttrPosAsWritten::Right;
}
-void DeclPrinter::prettyPrintAttributes(const Decl *D,
+// returns true if an attribute was printed.
+bool
@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten
getPosAsWritten(const Attr *A,
return DeclPrinter::AttrPosAsWritten::Right;
}
-void DeclPrinter::prettyPrintAttributes(const Decl *D,
+// returns true if an attribute was printed.
+bool
@@ -275,13 +278,15 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D,
if (Pos != AttrPosAsWritten::Left)
Out << ' ';
A->printPretty(Out, Policy);
+ hasPrinted = true;
if (Pos == AttrPosAsWritten::Left)
Out
https://github.com/erichkeane approved this pull request.
2 nits, else LGTM. Once you and @kimgr are in agreement with what to do next
(and have looked into my nits), feel free to commit.
https://github.com/llvm/llvm-project/pull/88600
___
https://github.com/erichkeane edited
https://github.com/llvm/llvm-project/pull/88600
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kimgr wrote:
> I am wondering if that'd be interesting and if so, maybe we can share it
> between both projects via upstreaming to Clang...
That sounds fantastic, but mostly because I don't have anything to offer, and
everything to benefit :)
I was just thinking about adding a
kimgr wrote:
Current PR passes all my tests, both Clang (`ninja ASTTests`), Clangd (`ninja
ClangdTests`) and IWYU end-to-end tests -- thanks!
https://github.com/llvm/llvm-project/pull/88600
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vgvassilev wrote:
> * IWYU uses `PolishForDeclaration` to get a valid _declaration_, and then
> does [some very hacky heuristic
> post-processing](https://github.com/include-what-you-use/include-what-you-use/blob/125341c412ceee9233ece8973848b49e770a9b82/iwyu_output.cc#L469)
> to turn it into
@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten
getPosAsWritten(const Attr *A,
return DeclPrinter::AttrPosAsWritten::Right;
}
-void DeclPrinter::prettyPrintAttributes(const Decl *D,
+// returns true if an attribute was printed.
+bool
kimgr wrote:
> If the intent is to produce a forward declaration the final keyword cannot be
> attached to a forward declaration. So I am not sure what's the "right" fix
> here...
I don't believe that's the intent of `DeclPrinter` or `PolishForDeclaration` --
* Clangd uses
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/88600
>From 9b2bb9068cbefcfffd0931fbbee46b1a0f536a4f Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double attribute printing of the
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/88600
>From cb3da95dd80c5ed991c5342e655e0f170eab16eb Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double attribute printing of the
vgvassilev wrote:
I've put a fix that fixes the cases that you mentioned...
https://github.com/llvm/llvm-project/pull/88600
___
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/88600
>From c68344d2d2f22f88ef386f655cc7698759fb551c Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double attribute printing of the
vgvassilev wrote:
IIUC, the PolishForDeclaration option is supposed to `When true, do certain
refinement needed for producing proper declaration tag; such as, do not print
attributes attached to the declaration. `. If the intent is to produce a
forward declaration the `final` keyword cannot
kimgr wrote:
Thank you.
I believe this should cover both cases, added some attempt at rationale in
comments:
```diff
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp
b/clang/unittests/AST/DeclPrinterTest.cpp
index f2b027a25621..8691a6c38f16 100644
---
vgvassilev wrote:
> > With .PolishForDeclaration=true, there are NO final specifiers (which is
> > what we want to produce forward decls in IWYU)
>
> This is actually a regression in this PR, and it breaks the clangd test added
> here:
>
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/88600
>From bcd04db735a78b4d7df93e88229ea4e2491fc09e Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double attribute printing of the
kimgr wrote:
> With .PolishForDeclaration=true, there are NO final specifiers (which is what
> we want to produce forward decls in IWYU)
This is actually a regression in this PR, and it breaks the clangd test added
here:
kimgr wrote:
Thanks!
Could you add this DeclPrinterTest unittest for regression?
```
TEST(DeclPrinter, TestTemplateFinal) {
ASSERT_TRUE(PrintedDeclCXX11Matches(
"template"
"class FinalTemplate final {};",
classTemplateDecl(hasName("FinalTemplate")).bind("id"),
"template class
@@ -6,11 +6,11 @@ typedef vector float3;
RWBuffer Buffer;
// expected-error@+2 {{class template 'RWBuffer' requires template arguments}}
-// expected-note@*:* {{template declaration from hidden source: template
class RWBuffer final}}
+// expected-note@*:* {{template
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/88600
>From b43c52e0ab76815e745d474c40944b4de1a929ab Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double attribute printing of the
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-hlsl
Author: Vassil Vassilev (vgvassilev)
Changes
Fixes #56517.
cc: @kimgr
---
Full diff: https://github.com/llvm/llvm-project/pull/88600.diff
4 Files Affected:
- (modified) clang/lib/AST/DeclPrinter.cpp (+18-13)
-
https://github.com/vgvassilev created
https://github.com/llvm/llvm-project/pull/88600
Fixes #56517.
cc: @kimgr
>From fe2622816265cf4977410d38dfd32d19df8eff5e Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Sat, 13 Apr 2024 06:39:34 +
Subject: [PATCH] Fix the double space and double
28 matches
Mail list logo
