[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

In D144709#4193603 , @MyDeveloperDay 
wrote:

> In D144709#4188921 , 
> @AlexanderHederstaf wrote:
>
>> What is the next step in the process? Anything I should do?
>
> If you have commit access you can go ahead and commit, if not and you want 
> one of us to do it, we need your name and email address

I don't have commit access. Please go ahead if you'd wish to do it for me. 
Should I share my email in some other forum than posting it in this comment 
section?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

Okay thanks. If the + options works let's go with
alexanderhederstaf+l...@gmail.com

Thanks again for the review and answers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

> Highlight by me. That is not acceptable, running the output of `clang-format` 
> through `clang-format` shall not result in any change. In that case it's 
> better to not touch it in any way.

That is no longer an issue in the latest patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500403.
AlexanderHederstaf added a comment.

Add comments for different cases.
Add more tests.
Fix whitespace issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -353,6 +358,10 @@
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -581,6 +590,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -590,6 +606,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};
 
@@ -599,6 +621,13 @@
   verifyFormat("volatile const int a;", "int volatile const a;", Style);
   verifyFormat("volatile const int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("volatile const Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("volatile const Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -608,6 +637,12 @@
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Custom;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -616,6 +651,12 @@
   verifyFormat("int volatile const a;", "int const volatile a;", Style);
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
+
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile co

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

Thanks for the review so far!

Passes all tests and works in one shot on my test files as well as 3k+ files 
repository, i.e. it does not have to be run twice.

As one use-case of this rule is to be able to checkout code and freely swap 
between left/right. Then the left alignment would also need to handle the same 
cases as right alignment.

I should also make templates with requires work, though I am not familiar with 
the variations that syntax may have.

@rymiel Good, this might take a few more iterations.




Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:226
+
+  const bool IsEastQualifier = PreviousCheck && [PreviousCheck]() {
+if (PreviousCheck->is(tok::r_paren)) {

HazardyKnusperkeks wrote:
> In the initial commit concerns were raised that `East` and `West` are 
> westcentric terms and should not be used. Thus you should stick here to 
> `Left` and `Right` the same terms we use in the configuration.
Replaced with right/left



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:228
+if (PreviousCheck->is(tok::r_paren)) {
+  return true;
+} else if (PreviousCheck->is(TT_TemplateCloser)) {

HazardyKnusperkeks wrote:
> Could you add comments what kind of source code we would have here?
Added comments for the different source code we may have.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:229
+  return true;
+} else if (PreviousCheck->is(TT_TemplateCloser)) {
+  return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);

HazardyKnusperkeks wrote:
> No `else` after `return`.
removed 'else' preceeding return.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:230
+} else if (PreviousCheck->is(TT_TemplateCloser)) {
+  return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
+} else if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier,

HazardyKnusperkeks wrote:
> That may be null, or not?
I assumed that if the token is identified as a template closer then there will 
exist an opener. As far as I am aware, something must preceed that opener.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve left to right const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500697.
AlexanderHederstaf added a comment.

Rebase after change to pointer to member was merged.
Add case for typename used for nested dependent names.
Add assertions for template closers/openers.

Add commented-out tests for two cases where right to left
produces code that does not compile.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -353,6 +358,10 @@
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -418,6 +427,15 @@
   verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
 
+  // Multiple template parameters.
+  verifyFormat("Bar", "Bar", Style);
+  // Variable declaration based on template type.
+  verifyFormat("Bar bar", "Bar bar", Style);
+
+  // Using typename for a nested dependent type name
+  verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;",
+   Style);
+
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
 
@@ -573,6 +591,20 @@
   verifyFormat("const std::Foo < int", "const std::Foo", "const std::Foo", Style);
 
+  // TODO: Fails to move const past std::Foo as it's not the last template
+  // parameter.
+  // Multiple template parameters.
+  // verifyFormat("Bar", "Bar", Style);
+
+  // TODO: Fails with "std::const Foo" when going left.
+  // Variable declaration based on template type.
+  // verifyFormat("Bar bar", "Bar bar", Style);
+
+  // TODO: Fails to move const past typename "typename const Foo::iterator".
+  // Using typename for a dependent name
+  // verifyFormat("const typename Foo::iterator", "typename Foo::iterator
+  // const", Style);
+
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
 
@@ -601,6 +633,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -610,6 +649,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};
 
@@ -619,6 +664,13 @@
   verifyFormat("volatile const int a;", "int volatile const a;", Style);
   verifyFormat("volatile const int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("volatile const Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo cons

[PATCH] D144709: [clang-format] Improve left to right const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf marked 6 inline comments as done.
AlexanderHederstaf added a comment.

Fixed comments and rebased. Tested right to left on the output of left to right 
and discovered two cases where the output code would not compile. Added new 
tests for those cases and I am working on improvements for right to left.




Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:231-232
+// The case  `Foo() const volatile override` -> `Foo() const volatile
+// override` The case  `Foo() volatile const final` -> `Foo() const 
volatile
+// final`
+if (PreviousCheck->is(tok::r_paren))

HazardyKnusperkeks wrote:
> `clang-format` did wrap your comment ;)
Re-arranged the comments.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:230
+} else if (PreviousCheck->is(TT_TemplateCloser)) {
+  return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
+} else if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier,

HazardyKnusperkeks wrote:
> AlexanderHederstaf wrote:
> > HazardyKnusperkeks wrote:
> > > That may be null, or not?
> > I assumed that if the token is identified as a template closer then there 
> > will exist an opener. As far as I am aware, something must preceed that 
> > opener.
> Then please add an assertion. So we would get a nice message if your 
> assumption is wrong.
Added assertions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve left to right const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500710.
AlexanderHederstaf added a comment.

Fix simple types not moving all the way right.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -350,9 +355,24 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -418,6 +438,15 @@
   verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
 
+  // Multiple template parameters.
+  verifyFormat("Bar", "Bar", Style);
+  // Variable declaration based on template type.
+  verifyFormat("Bar bar", "Bar bar", Style);
+
+  // Using typename for a nested dependent type name
+  verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;",
+   Style);
+
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
 
@@ -573,6 +602,20 @@
   verifyFormat("const std::Foo < int", "const std::Foo", "const std::Foo", Style);
 
+  // TODO: Fails to move const past std::Foo as it's not the last template
+  // parameter.
+  // Multiple template parameters.
+  // verifyFormat("Bar", "Bar", Style);
+
+  // TODO: Fails with "std::const Foo" when going left.
+  // Variable declaration based on template type.
+  // verifyFormat("Bar bar", "Bar bar", Style);
+
+  // TODO: Fails to move const past typename "typename const Foo::iterator".
+  // Using typename for a dependent name
+  // verifyFormat("const typename Foo::iterator", "typename Foo::iterator
+  // const", Style);
+
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
 
@@ -601,6 +644,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -610,6 +660,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};

[PATCH] D144709: [clang-format] Improve left to right const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500792.
AlexanderHederstaf added a comment.

Update right to left analyzer
Add support for requires clauses
Ignore usages of struct/class keyword

Add new, and uncomment old tests
Fix invalid tests with 'const Foo b* const;'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,24 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -418,7 +439,41 @@
   verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
 
-  // don't adjust macros
+  // Multiple template parameters.
+  verifyFormat("Bar", "Bar", Style);
+  // Variable declaration based on template type.
+  verifyFormat("Bar bar", "Bar bar", Style);
+
+  // Using typename for a nested dependent type name.
+  verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;",
+   Style);
+
+  // Doesn't move past C-style struct/class.
+  verifyFormat("void foo(const struct A a);", "void foo(const struct A a);",
+   Style);
+  verifyFormat("void foo(const class A a);", "void foo(const class A a);",
+   Style);
+
+  /

[PATCH] D144709: [clang-format] Improve left to right const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

There are a lot of complicated cases when trying to extend this functionality. 
I addressed a few more and thought that it might be acceptable to avoid 
adjusting the qualifiers for variable declared as

  struct Foo const; -> const struct Foo;
  const struct Foo; -> struct Foo const;
  
  const struct {
int a;
  } foo = {5};
   -> 
  struct {
int a;
  } const foo = {5};

as they are difficult to distinguish from

  const struct Foo {
int a;
  }

where this is not valid code

  struct const Foo {
int a;
  }

I have still not tested left->right->left on the llvm codebase, I'll update you 
when it's done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500799.
AlexanderHederstaf added a comment.

Update commit message


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,24 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -418,7 +439,41 @@
   verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
 
-  // don't adjust macros
+  // Multiple template parameters.
+  verifyFormat("Bar", "Bar", Style);
+  // Variable declaration based on template type.
+  verifyFormat("Bar bar", "Bar bar", Style);
+
+  // Using typename for a nested dependent type name.
+  verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;",
+   Style);
+
+  // Doesn't move past C-style struct/class.
+  verifyFormat("void foo(const struct A a);", "void foo(const struct A a);",
+   Style);
+  verifyFormat("void foo(const class A a);", "void foo(const class A a);",
+   Style);
+
+  // Doesn't move past struct/class combined declaration and variable
+  // definition.
+  verifyFormat("const struct {\n} var;", "const struct {\n} var;", Style

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 501137.
AlexanderHederstaf added a comment.

Simplify insertQualifierAfter.
Ignore decltype, typeof, and _Atomic.
Set length of TT_TemplateCloser to 1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,24 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -418,7 +439,49 @@
   verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
 
-  // don't adjust macros
+  // Multiple template parameters.
+  verifyFormat("Bar", "Bar", Style);
+  // Variable declaration based on template type.
+  verifyFormat("Bar bar", "Bar bar", Style);
+
+  // Using typename for a nested dependent type name.
+  verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;",
+   Style);
+
+  // Don't move past C-style struct/class.
+  verifyFormat("void foo(const struct A a);", "void foo(const struct A a);",
+   Style);
+  verifyFormat("void foo(const class A a);", "void foo(const class A a);",
+   Style);
+
+  // Don't move past struct/class combin

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

Discovered incorrect code generation for  `llvm/include/llvm/ADT/StringMap.h`.

  using base = StringMapIterBase,
 const StringMapEntry>;

was converted to, removing two characters. I checked the Token length in 
InsertQualifierAfter and found that it was not 1. How should I proceed with 
this particular issue @MyDeveloperDay

  using base = StringMapIterBase,
 StringMapEntry const




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 501420.
AlexanderHederstaf added a comment.

Distinguish configured qualifiers from other qualifiers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,44 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  // static is not configured, unchanged on the left of the right hand
+  // qualifiers.
+  verifyFormat("int static const volatile;", "volatile const int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const volatile int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const int volatile static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "volatile const Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const volatile Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const Foo volatile static;",
+   Style);
+
+  verifyFormat("Foo inline static const;", "const Foo inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo const inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline const static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline static const;", Style);
+
+  verifyFormat("Foo::Bar const volatile A::*;",
+ 

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

This code from llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp failed 
with **Right** but without a full list of specifiers. As **static** was not 
configured, the } was not discovered by **IsRightQualifier**. Changed some of 
the qualifier tests to check against the full list instead.

  struct ExecutionClass {
ExecutionMode Mask;
const char *Description;
  } static const kExecutionClasses[] = {
  {ExecutionMode::ALWAYS_SERIAL_IMPLICIT_REGS_ALIAS |
   ExecutionMode::ALWAYS_SERIAL_TIED_REGS_ALIAS,
   "Repeating a single implicitly serial instruction"},
  {ExecutionMode::SERIAL_VIA_EXPLICIT_REGS,
   "Repeating a single explicitly serial instruction"},
  {ExecutionMode::SERIAL_VIA_MEMORY_INSTR |
   ExecutionMode::SERIAL_VIA_NON_MEMORY_INSTR,
   "Repeating two instructions"},
  };

Resulted in

  } static kExecutionClasses const[] = {

Which is now fixed. This struct will be left unchanged as with other structs.

I ran clang format on all files in clang/ and llvm/, where I had added 
QualifierAlignment: Right to any .clang-format. I built with

  cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=Release 
-DLLVM_ENABLE_PROJECTS="clang"
  cmake --build build

Is that sufficient or did you imagine some other build?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

In D144709#4160820 , @MyDeveloperDay 
wrote:

> As tests currently fail can you set it to "planned changes" until they pass
>
>   [  FAILED  ] 5 tests, listed below:
>   [  FAILED  ] QualifierFixerTest.RightQualifier
>   [  FAILED  ] QualifierFixerTest.LeftQualifier
>   [  FAILED  ] QualifierFixerTest.ConstVolatileQualifiersOrder
>   [  FAILED  ] QualifierFixerTest.MoveConstBeyondType
>   [  FAILED  ] QualifierFixerTest.TemplatesRight

Tests should pass now, but how/where do I set that setting?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 501476.
AlexanderHederstaf added a comment.

Change order for typename and :: as
const typename ::Bar b; is valid code.

Add code for handling ::template.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,44 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  // static is not configured, unchanged on the left of the right hand
+  // qualifiers.
+  verifyFormat("int static const volatile;", "volatile const int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const volatile int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const int volatile static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "volatile const Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const volatile Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const Foo volatile static;",
+   Style);
+
+  verifyFormat("Foo inline static const;", "const Foo inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo const inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline const static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline static const;", Style);
+
+ 

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

In D144709#4160939 , 
@AlexanderHederstaf wrote:

> This code from llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp failed 
> with **Right** but without a full list of specifiers. As **static** was not 
> configured, the } was not discovered by **IsRightQualifier**. Changed some of 
> the qualifier tests to check against the full list instead.
>
>   struct ExecutionClass {
> ExecutionMode Mask;
> const char *Description;
>   } static const kExecutionClasses[] = {
>   {ExecutionMode::ALWAYS_SERIAL_IMPLICIT_REGS_ALIAS |
>ExecutionMode::ALWAYS_SERIAL_TIED_REGS_ALIAS,
>"Repeating a single implicitly serial instruction"},
>   {ExecutionMode::SERIAL_VIA_EXPLICIT_REGS,
>"Repeating a single explicitly serial instruction"},
>   {ExecutionMode::SERIAL_VIA_MEMORY_INSTR |
>ExecutionMode::SERIAL_VIA_NON_MEMORY_INSTR,
>"Repeating two instructions"},
>   };
>
> Resulted in
>
>   } static kExecutionClasses const[] = {
>
> Which is now fixed. This struct will be left unchanged as with other structs.
>
> I used clang-format on all files in clang/ and llvm/, where I had added 
> QualifierAlignment: Right to any .clang-format. I built with
>
>   cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=Release 
> -DLLVM_ENABLE_PROJECTS="clang"
>   cmake --build build
>
> Is that sufficient or did you imagine some other build?

Performed the test for
Original -> Right -> compiles


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

Thanks for the comments and help on this review. There are a lot of special 
cases I had not considered before, and while it works on the clang/ and llvm/ 
subfolders as well as tests and the other repository I tested on, I suppose 
there could still be some hidden issues. Allowing any identifier without the 
requirement that it is a pointer type adds a lot of complexity.

It sounds good to close the existing issues, let this version live some time 
and watch out for new issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added inline comments.



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:733
+  // TODO: Something strange is going on with this formating.
+  verifyFormat("Bar < Foo, const Foo >> ;", "Bar < Foo, Foo const >> ;", 
Style);
+

rymiel wrote:
> This... isn't valid syntax? an opening < is missing.
> Perhaps you could adapt https://github.com/llvm/llvm-project/issues/56111 
> into a test, it seems to be fixed by the patch, but there might still be 
> corner-cases
Right, I must've focused too much on the missing right >. What is the procedure 
now that the review is accepted. Can I still upload a fix to this and to 
include that issue in the tests?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 502545.
AlexanderHederstaf added a comment.

Add extra tests based on https://github.com/llvm/llvm-project/issues/56111


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,44 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  // static is not configured, unchanged on the left of the right hand
+  // qualifiers.
+  verifyFormat("int static const volatile;", "volatile const int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const volatile int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const int volatile static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "volatile const Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const volatile Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const Foo volatile static;",
+   Style);
+
+  verifyFormat("Foo inline static const;", "const Foo inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo const inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline const static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline static const;", Style);
+
+  verifyFormat("Foo::Bar const volat

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf planned changes to this revision.
AlexanderHederstaf added a comment.

Missed right->left in the fix. Also looking a bit at comments, e.g.

  ns::/*ns2::*/vector v;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 502563.
AlexanderHederstaf added a comment.
This revision is now accepted and ready to land.

Handle /*c*/ comments inside types.
Add >> test to right->left.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -241,7 +241,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("const Foo a;", Style);
@@ -252,7 +252,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
 
   verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
   verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
@@ -275,7 +275,7 @@
   verifyFormat("int const *b;", Style);
   verifyFormat("int const &b;", Style);
   verifyFormat("int const &&b;", Style);
-  verifyFormat("int const *b const;", Style);
+  verifyFormat("int const *const b;", Style);
   verifyFormat("int *const c;", Style);
 
   verifyFormat("Foo const a;", Style);
@@ -286,7 +286,7 @@
   verifyFormat("Foo const *b;", Style);
   verifyFormat("Foo const &b;", Style);
   verifyFormat("Foo const &&b;", Style);
-  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo const *const b;", Style);
   verifyFormat("Foo *const b;", Style);
   verifyFormat("Foo const *const b;", Style);
   verifyFormat("auto const v = get_value();", Style);
@@ -303,6 +303,11 @@
   verifyFormat("void foo() const final;", Style);
   verifyFormat("void foo() const final LLVM_READONLY;", Style);
   verifyFormat("void foo() const LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile override;", Style);
+  verifyFormat("void foo() const volatile override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile final;", Style);
+  verifyFormat("void foo() const volatile final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const volatile LLVM_READONLY;", Style);
 
   verifyFormat(
   "template  explicit Action(Action const &action);",
@@ -343,6 +348,7 @@
   verifyFormat("int const volatile;", "volatile const int;", Style);
   verifyFormat("int const volatile;", "const volatile int;", Style);
   verifyFormat("int const volatile;", "const int volatile;", Style);
+
   verifyFormat("int const volatile *restrict;", "volatile const int *restrict;",
Style);
   verifyFormat("int const volatile *restrict;", "const volatile int *restrict;",
@@ -350,9 +356,44 @@
   verifyFormat("int const volatile *restrict;", "const int volatile *restrict;",
Style);
 
+  verifyFormat("long long int const volatile;", "const long long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long const long int volatile;",
+   Style);
+  verifyFormat("long long int const volatile;", "long long volatile int const;",
+   Style);
+  verifyFormat("long long int const volatile;", "long volatile long const int;",
+   Style);
+  verifyFormat("long long int const volatile;", "const long long volatile int;",
+   Style);
+
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  // static is not configured, unchanged on the left of the right hand
+  // qualifiers.
+  verifyFormat("int static const volatile;", "volatile const int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const volatile int static;",
+   Style);
+  verifyFormat("int static const volatile;", "const int volatile static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "volatile const Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const volatile Foo static;",
+   Style);
+  verifyFormat("Foo static const volatile;", "const Foo volatile static;",
+   Style);
+
+  verifyFormat("Foo inline static const;", "const Foo inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo const inline static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline const static;", Style);
+  verifyFormat("Foo inline static const;", "Foo inline static const;", Style);
+

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf requested review of this revision.
AlexanderHederstaf added a comment.

Refactored using the NonComment() functions and added some tests to verify that 
it works correctly.

In D144709#4171029 , @MyDeveloperDay 
wrote:

> I might have been tempted to handle the comments in a separate commit, I 
> don't like to do too much in one go or the review gets hard to manage.

I had the same thought but as the comments change is not only an extension of 
the capability I included it to be safe. Without the change it would lead to 
code that doesn't compile.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve QualifierAlignment

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

What is the next step in the process? Anything I should do?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf created this revision.
Herald added a project: All.
AlexanderHederstaf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Qualifiers were not moved for non-pointer non-simple types.
  Remove the requirement for *,&,&& and add extra checks for
  previous tokens. Further simplify the logic for types by
  additional requirements on tokens preceeding the current
  test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp

Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -217,62 +217,61 @@
   if (LeftRightQualifierAlignmentFixer::isPossibleMacro(Tok->Next))
 return Tok;
 
-  auto AnalyzeTemplate =
-  [&](const FormatToken *Tok,
-  const FormatToken *StartTemplate) -> const FormatToken * {
-// Read from the TemplateOpener to TemplateCloser.
-FormatToken *EndTemplate = StartTemplate->MatchingParen;
-if (EndTemplate) {
-  // Move to the end of any template class members e.g.
-  // `Foo::iterator`.
-  if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
-  tok::identifier)) {
-EndTemplate = EndTemplate->Next->Next;
-  }
-}
-if (EndTemplate && EndTemplate->Next &&
-!EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
-  insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
-  // Remove the qualifier.
-  removeToken(SourceMgr, Fixes, Tok);
-  return Tok;
+  const FormatToken *PreviousCheck = Tok->Previous;
+  while (PreviousCheck && llvm::is_contained(ConfiguredQualifierTokens,
+ PreviousCheck->Tok.getKind())) {
+PreviousCheck = PreviousCheck->Previous;
+  }
+
+  const bool IsEastQualifier = PreviousCheck && [PreviousCheck]() {
+if (PreviousCheck->is(tok::r_paren)) {
+  return true;
+} else if (PreviousCheck->is(TT_TemplateCloser)) {
+  return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
+} else if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier,
+  tok::kw_auto)) {
+  return true;
 }
-return nullptr;
-  };
+return false;
+  }();
 
-  FormatToken *Qual = Tok->Next;
-  FormatToken *LastQual = Qual;
-  while (Qual && isQualifierOrType(Qual, ConfiguredQualifierTokens)) {
-LastQual = Qual;
-Qual = Qual->Next;
+  const FormatToken *LastQual = Tok;
+  while (LastQual->Next && llvm::is_contained(ConfiguredQualifierTokens,
+  LastQual->Next->Tok.getKind())) {
+LastQual = LastQual->Next;
+  }
+
+  if (IsEastQualifier) {
+if (LastQual != Tok)
+  rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
+return Tok;
   }
-  if (LastQual && Qual != LastQual) {
-rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
-Tok = LastQual;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier,
- TT_TemplateCloser)) {
-FormatToken *Closer = Tok->Next->Next;
-rotateTokens(SourceMgr, Fixes, Tok, Tok->Next, /*Left=*/false);
-Tok = Closer;
+
+  const FormatToken *TypeToken = LastQual->Next;
+  if (!TypeToken)
 return Tok;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier,
- TT_TemplateOpener)) {
-// `const ArrayRef a;`
-// `const ArrayRef &a;`
-const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next);
-if (NewTok)
-  return NewTok;
-  } else if (Tok->startsSequence(QualifierType, tok::coloncolon,
- tok::identifier, TT_TemplateOpener)) {
-// `const ::ArrayRef a;`
-// `const ::ArrayRef &a;`
-const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next->Next);
-if (NewTok)
-  return NewTok;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier) ||
- Tok->startsSequence(QualifierType, tok::coloncolon,
- tok::identifier)) {
-FormatToken *Next = Tok->Next;
+
+  if (TypeToken->isSimpleTypeSpecifier()) {
+const FormatToken *LastSimpleTypeSpecifier = TypeToken;
+while (LastSimpleTypeSpecifier->Next &&
+   LastSimpleTypeSpecifier->Next->isSimpleTypeSpecifier()) {
+  LastSimpleTypeSpecifier = LastSimpleTypeSpecifier->Next;
+}
+rotateTokens(SourceMgr, Fixes, Tok, LastSimpleTypeSpecifier,
+ /*Left=*/false);
+return LastSimpleTypeSpecifier;
+  } else if (PreviousCheck && PreviousCheck->isSimpleTypeSpecifier()) {
+if (LastQual != Tok)
+  rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
+return Tok;
+  }
+
+  if (TypeToken->is(tok::co

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a reviewer: MyDeveloperDay.
AlexanderHederstaf added a comment.

I tried to refactor QualifierAlignmentFixer to handle more types, notably 
removing the *,&,&& requirement as e.g. const Foo did not work. To avoid moving 
qualifiers too far, a test for "already east const" is introduced. However, to 
get the correct order for the east qualifiers Clang-Format must be used twice. 
I assume it's related to how fixes/replacements are handled. Would you like to 
have a look and perhaps suggest any improvements?

As it must be used twice, some tests related to volatile/const order fail, but 
I noticed most of them would also fails previously if the type is not simple.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

I will add new tests / examples as well as look into the volatile/const 
ordering.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500215.
AlexanderHederstaf added a comment.

Fixed the const/volatile placement. Added a few tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -581,6 +581,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -590,6 +597,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};
 
@@ -599,6 +612,13 @@
   verifyFormat("volatile const int a;", "int volatile const a;", Style);
   verifyFormat("volatile const int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("volatile const Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("volatile const Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -608,6 +628,12 @@
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Custom;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -616,6 +642,12 @@
   verifyFormat("int volatile const a;", "int const volatile a;", Style);
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
+
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
 }
 
 TEST_F(QualifierFixerTest, InlineStatics) {
@@ -704,6 +736,21 @@
 
   verifyFormat("static inline int const volatile *a const;",
"const int inline static  volatile *a const;", Style);
+
+  verifyFormat("static inline Foo const volatile a;",
+   "const inline static volatile Foo a;", Style);
+  verifyFormat("static inline Foo const volatile a;",
+   "volatile inline static const Foo a;", Style);
+  // TODO: Fails as static or inline can't be moved past Foo to the left.
+  // verifyFormat("static inline Foo const volatile a;",
+  //  "Foo const inline static  volatile a;", Style);
+  // verifyFormat("static inline Foo const volatile a;",
+  //  "Foo volatile inline static  const a;", Style);
+  // verifyFormat("static inline Foo const volatile a;",
+  //  "const Foo inline static  volatile a;", Style);
+
+  // verifyFormat("static inline Foo const volatile *a const;",
+  //  "const Foo inline sta

[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf updated this revision to Diff 500225.
AlexanderHederstaf added a comment.

Add test with advanced type that would not work before.
Add pointer to member.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144709/new/

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -353,6 +353,10 @@
   verifyFormat("static int const bat;", "static const int bat;", Style);
   verifyFormat("static int const bat;", "static int const bat;", Style);
 
+  verifyFormat("Foo::Bar const volatile A::*;",
+   "volatile const Foo::Bar A::*;",
+   Style);
+
   verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;",
Style);
   verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;",
@@ -581,6 +585,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -590,6 +601,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};
 
@@ -599,6 +616,13 @@
   verifyFormat("volatile const int a;", "int volatile const a;", Style);
   verifyFormat("volatile const int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("volatile const Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("volatile const Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -608,6 +632,12 @@
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Custom;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -616,6 +646,12 @@
   verifyFormat("int volatile const a;", "int const volatile a;", Style);
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
+
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
 }
 
 TEST_F(QualifierFixerTest, InlineStatics) {
@@ -704,6 +740,21 @@
 
   verifyFormat("static inline int const volatile *a const;",
"const int inline static  volatile *a const;", Style);
+
+  verifyFormat("static inline Foo const volatile a;",
+   "const inline static volatile Foo a;", Style);
+  verifyFormat("static inline Foo const volatile a;",
+   "volatile inline static const Foo a;", Style);
+  // TODO: Fails as static or inline can't be moved past Foo to the left.