[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Yes, here's my reply: https://bugs.llvm.org/show_bug.cgi?id=47161#c2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D79773#2434126 , @JohelEGP wrote:

> I was a bit late, but thanks for everything!

No problem, sorry not for waiting, but let the games begin.

P.S. I think I may have left a comment for you over in one of your bugs. not 
sure if you saw that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

I was a bit late, but thanks for everything!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG840e651dc6d7: [clang-format] Improve clang-formats handling 
of concepts (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@JohelEGP can you let me know if you plan to remove the "requested changes" you 
asked for back in July?

I cannot commit with the revision until that is removed


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

I am in favor of landing this and iterating.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D79773#2432088 , @miscco wrote:

> As someone who has extensively worked with concepts I cannot stress how much 
> this would improve my live

and I'd like to think that this will help even in its current form, but it 
needs someone to accept it first before it can land,

This has been my problem with clang-format , I do try and give some of my time 
to keeping up with the new and outstanding reviews coming in, but my own 
progress on clang-format or the fixing of bugs has slowed up because of a lack 
of people to review, we just need a couple more people who are willing to come 
once or twice a week and just keep things rolling along. but someone has to 
"watch the watcher" too.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

As someone who has extensivly worked with conscepts I cannot stress how much 
this would improve my live


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Having reviewed the current status I'm going to leave this patch with just 
minor changes from where its been for some time, if others feel it has merits 
in landing as experimental support I'd be happy with that. I would rather build 
from a base than try and keep rebuilding the same tower.

The concepts support in clang-format is currently poor, actually, it's 
downright destructive, I've tried to cover the basic concept usage and I'm 
happy to support this feature against individual bug report where I can.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309350.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

Address the issue with still munching the semi
mark as experimental in the release notes


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

klimek wrote:
> MyDeveloperDay wrote:
> > miscco wrote:
> > > miscco wrote:
> > > > I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> > > > /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
> > > To be more specific, I am concerned what happens if you have multiple 
> > > template arguments where a linebreak should occur. Is this still 
> > > happening here?
> > > 
> > > 
> > > ```
> > > template 
> > > concept someVeryLongConceptName = 
> > > someVeryLongConstraintName1;
> > > ```
> > This is formatted as
> > 
> > ```
> > template 
> > concept someVeryLongConceptName =
> > someVeryLongConstraintName1;
> > ```
> This seems like a very detailed way to parse them; generally, we try to only 
> parse rough structure here.
So whilst I tend to agree, and I've tried to write this loop to be less and it 
just didn't seem to catch the cases that had already been given in the unit 
tests,

This ended up being a lot likes its own parseStructuralElement, I think the 
difference here is that I've written it as a series of if's rather than a 
switch statement.

I feel I'd be happier to push the patch through then address individual 
specific cases



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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 309335.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

rebase before making any further changes


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

It would be maybe possible to:

- address @klimek 's comment and do any other necessary cleanup
- create a bug ticket for what @JohelEGP found with ctors
- mark clang-format's support of concept as experimental?

This patch is getting big IMO and I really think that something is better than 
nothing in this case.
And at least we could parallelise the efforts on the remaining (edge) cases 
when this gets landed.
WDYT?


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I don't think it was far off, I just I agreed with @klimek in trying to address 
the many different cases that came up I started to feel that  
`parseConstraintExpression` was becoming fragile

Having said that I like to think because they are in concepts specific parse 
code I won't break too much (famous last words!), any if you've tried to format 
concept code with the current clang-format well good luck with that.

Perhaps something is better than nothing?

if you think it has merits, if it covers most cases ok, then I'd be happy to 
see it land and work on any refinements from there.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@MyDeveloperDay, is there anything I can do to help you on that?

BTW, libc++ would like to use clang-format (more than it does currently) and 
having concepts support would be awesome as there are more and more 
concept-related patches coming in.
IMHO, even support for most common cases would be a great improvement.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 300489.
MyDeveloperDay added a comment.

Rebase the patch


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14044,6 +14044,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14055,6 +14056,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17206,6 +17208,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Foo>() && Bar> {\n"
+   "  //\n"
+ 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Could you do a rebase? The patch no longer applies:

  error: patch failed: clang/docs/ClangFormatStyleOptions.rst:2711
  error: clang/docs/ClangFormatStyleOptions.rst: patch does not apply
  error: patch failed: clang/docs/ReleaseNotes.rst:398
  error: clang/docs/ReleaseNotes.rst: patch does not apply
  error: patch failed: clang/lib/Format/FormatToken.h:470
  error: clang/lib/Format/FormatToken.h: patch does not apply
  error: patch failed: clang/lib/Format/TokenAnnotator.cpp:1337
  error: clang/lib/Format/TokenAnnotator.cpp: patch does not apply


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

In D79773#2141164 , @MyDeveloperDay 
wrote:

> This is actually failing in the handling of parsing the `<>`  in 
> (`parseAngle()` actually `parseBrace()` inside of that), it is not really 
> concepts specific except it may be seen now because of the presence of the `;`

I opened LLVM 47161  to defer this 
to upstream.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

I found another case. See how everything after the //requires-clause// is 
indented for constructors with just the constructor's name (it works otherwise, 
maybe because it looks like a function).

  class [[nodiscard]] data_t
  {
  public:
  template 
  requires std::same_as, 
flippable_tile_id>
  /*explicit*/ data_t(R&& ids, const format_t f = encoding_t::csv)
: format_t{f}, tile_ids_{ranges::to_vector(std::forward(ids))}
  {
  }
  
  [[nodiscard]] bool operator==(const data_t&) const = default;
  };


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:631-636
+  else if (FormatTok->is(tok::arrow)) {
+// Following the } we can find a trailing return type arrow
+// as part of an implicit conversion constraint.
+nextToken();
+parseStructuralElement();
+  }

I'd have expected this to be just in front of line 629 (if... tok::semi) - does 
that break something? Don't we want to munch the semi after this?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

MyDeveloperDay wrote:
> miscco wrote:
> > miscco wrote:
> > > I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> > > /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
> > To be more specific, I am concerned what happens if you have multiple 
> > template arguments where a linebreak should occur. Is this still happening 
> > here?
> > 
> > 
> > ```
> > template 
> > concept someVeryLongConceptName = 
> > someVeryLongConstraintName1;
> > ```
> This is formatted as
> 
> ```
> template 
> concept someVeryLongConceptName =
> someVeryLongConstraintName1;
> ```
This seems like a very detailed way to parse them; generally, we try to only 
parse rough structure here.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: klimek.
MyDeveloperDay added a comment.

This issue is caused by the presence of the `;` in the requires clause (`0)>;`) 
inside the template declaration

  template 
  requires std::invocable...>
  struct [[nodiscard]] constant : std::bool_constant < requires
  {
  typename _require_constant_<(std::invoke(F{}, Args{}()...), 0)>;
  } > {};

These below may not be legal C++ examples but show the same behaviour

  struct constant : Foo < typename {
  Foo;
  } > {};
  
  struct constant : Foo { };

This is actually failing in the handling of parsing the `<>`  in 
(`parseAngle()` actually `parseBrace()` inside of that), it is not really 
concepts specific except it may be seen now because of the presence of the `;`

The problem comes about I think because the line is broken by the ";" during 
the `parseBrace` at some point the `Next` token is null, I assume that's 
because the lines are broken by the ';' as such ultimately the < and > rather 
than being seen as TT_TemplateOpener and TT_TemplateCloser are seen as 
TT_BinaryOperators (by default when parseAngle fails)

I'm not sure how I could solve this, I might need help from a higher power 
@klimek


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Ah, that makes sense. The very definition of Allman is to always break before 
braces. I suppose I'll need to use `BreakBeforeBraces: Custom` and 
`BraceWrapping:`. I did some testing and noticed that the weird format comes 
with any of these:

  BreakBeforeBraces: Custom
  BraceWrapping:
AfterControlStatement: MultiLine

  BreakBeforeBraces: Custom
  BraceWrapping:
AfterFunction: true

Since a requires-expression is neither a control statement nor a function, I 
suppose it might eventually need its own `BraceWrapping` nested configuration 
flag. For now, I'd prefer if they never break.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

https://reviews.llvm.org/D79773#2131680 has something to do with your 
.clang-format file, the base LLVM style seems fine.

  template 
  concept bool EqualityComparable = requires(T a, T b) {
  { a == b } -> bool;
  };

vs

  template 
  concept bool EqualityComparable = requires(T a, T b)
  {
  {
  a == b
  } -> bool;
  };

From what I can tell its because of the "BreakBeforeBraces: Allman or GNU" 
styles.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Yes, it's valid C++ (is my hope). It's from WIP code, which I made available to 
show you: https://github.com/johelegp/jge.

Here it is building, running and passing the tests: 
https://travis-ci.com/github/johelegp/jge/jobs/358137355#L559.

Here are links to the above linked snippets:

- Now looking good:
  - 
https://github.com/johelegp/jge/blob/master/include/jge/cartesian.hpp#L84-L90
  - https://github.com/johelegp/jge/blob/master/include/jge/plane.hpp#L54-L59
  - https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L18-L20.
- Just reported:
  - Inheritance: 
https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L11-L21 
(https://reviews.llvm.org/D79773#2131680)
  - Non list-initialization: 
https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L11-L16 
(https://reviews.llvm.org/D79773#2132086)
  - For the `EqualityComparable` case 
(https://reviews.llvm.org/D79773#2131680), I use this macro: 
https://github.com/johelegp/jge/blob/master/include/jge/detail/quantity_wrappers.hpp#L4-L5.
 Example: 
https://github.com/johelegp/jge/blob/master/include/jge/detail/quantity_wrappers.hpp#L59.
 I also used it before the previous fixes besides `//` comments.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Thank you @JohelEGP please keep them coming, I'll try and update the patch as 
soon as I have some fixes, I kind of feel like we are addressing what would 
have just been downstream bugs anyway. (some are quite bizarre!) so lets squish 
as many of them as we can before we land this.

My assumption is you are giving me valid c++ code, if so that is great I'll try 
and address them, but your examples are probably beyond my own concepts 
knowledge. (This is actually why I started to add this to clang-format, I 
wanted a way to learn about concepts)


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Another weird formatting when direct-initializing a variable with a 
non-parenthesized requires-expression:

  constexpr void test_width()
  {
  bool invalid_rep{!requires {typename jge::width;
  }
  }
  ;
  }

vs

  constexpr void test_width()
  {
  bool invalid_rep{!(requires { typename jge::width; })};
  bool invalid_rep = !(requires { typename jge::width; });
  bool invalid_rep = !requires
  {
  typename jge::width;
  };
  }


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP requested changes to this revision.
JohelEGP added a comment.
This revision now requires changes to proceed.

Thank you. Everything I reported works fine now.

I have two more cases for now. First are your examples in the OP.
You have

  template 
  concept bool EqualityComparable = requires(T a, T b) {
{ a == b } -> bool;
  };

But this is what I get:

  template 
  concept bool EqualityComparable = requires(T a, T b)
  {
  {
  a == b
  } -> bool;
  };

Perhaps it's because I compiled against commit 
`71d88cebfb42c8c5ac2d54b42afdcca956e55660` (Fri Jul 3 13:46:41 2020 -0400).
Next:

  template 
  requires std::invocable...>
  struct [[nodiscard]] constant : std::bool_constant < requires
  {
  typename _require_constant_<(std::invoke(F{}, Args{}()...), 0)>;
  } > {};

There's spaces around the template brackets. The lack of indentation is also 
odd, considering that in this context things are indented.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 275356.
MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added a comment.

Addressed review comments
Added one or two more tests

NOTE: renamed the option from `AlwaysBreakBeforeConceptDeclarations`  -> 
`BreakBeforeConceptDeclarations`


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13648,6 +13648,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -13659,6 +13660,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16728,6 +16730,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:13540
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

curdeius wrote:
> MyDeveloperDay wrote:
> > curdeius wrote:
> > > Format: leading whitespace. Should it be in the same "group" as 
> > > `AlignConsecutiveMacros`?
> > I might be misunderstanding your comment, but the list should alphabetic 
> > but now I realize it should be after the `Allows`
> I just wanted to point out that when you don't put a blank line after 
> CHECK_PARSE_BOOL, then the next one gets indented. And yes, the order isn't 
> ok.  BTW, us "Always" in the name necessary? Cf. other "Break" options.
@curdeius I didn't get to make changes to match your previous review comments 
yet, but I 100% agree with you why did I put Always here!

I WILL remove that as I can see further down the road if this got changed to an 
enumeration it would be

```
AlwaysBreakBeforeConceptDeclarations = Always
AlwaysBreakBeforeConceptDeclarations = Sometimes
AlwaysBreakBeforeConceptDeclarations = Never
```

Which would be very confusing...

I'll address those concerns next



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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius marked an inline comment as done.
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:13540
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

MyDeveloperDay wrote:
> curdeius wrote:
> > Format: leading whitespace. Should it be in the same "group" as 
> > `AlignConsecutiveMacros`?
> I might be misunderstanding your comment, but the list should alphabetic but 
> now I realize it should be after the `Allows`
I just wanted to point out that when you don't put a blank line after 
CHECK_PARSE_BOOL, then the next one gets indented. And yes, the order isn't ok. 
 BTW, us "Always" in the name necessary? Cf. other "Break" options.



Comment at: clang/unittests/Format/FormatTest.cpp:16745
+   "::std::is_copy_constructable and "
+   "::std::is_move_constructable and\n"
+   "requires (T c) {\n"

MyDeveloperDay wrote:
> curdeius wrote:
> > Isn't it a strange indentation?
> there is no newline on the line above, but I clang-format the tests so it 
> wraps it, I know visually its confusing
O, ok, thanks for the explanation. It's indeed confusing in the review.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 275213.
MyDeveloperDay added a comment.

Still a work in progress but covers I think the case @JohelEGP found.


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13639,6 +13639,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveBitFields);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -13659,6 +13660,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16728,6 +16730,252 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Using parameter packs within non-parenthesized requires-clauses results in 
weird formatting:

  template 
  requires std::invocable < F, std::invoke_result_t
  ... > struct constant;

vs

  template 
  requires(std::invocable...>)
  struct constant;


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Thank you I see now... super subtle but I'll take a look.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

In D79773#2125040 , @MyDeveloperDay 
wrote:

> Just so I'm clear for this and your Allman coment would you show me what you 
> are seeing and what you expect to see? just so I understand.


Sure, thank you.

This is what I have, as shown above. Actually fixed to show surrounding 
context, which is relevant to formatting. Note the `//` comment:

  namespace jge
  {
  template 
  struct [[nodiscard]] size2d
  {
  template 
  friend constexpr auto
  operator*(const size2d& l, const units::quantity& r) noexcept(
  noexcept(l.w* r)) requires(requires { l.w* r; }) //
  {
  return size2d{l.w * r, l.h * r};
  }
  };
  
  } // namespace jge

I added `//` to make the formatting look like what I want. If I remove it, this 
is what I get:

  namespace jge
  {
  template 
  struct [[nodiscard]] size2d
  {
  template 
  friend constexpr auto
  operator*(const size2d& l, const units::quantity& r) noexcept(
  noexcept(l.w* r)) requires(requires { l.w* r; }) {
  return size2d{l.w * r, l.h * r};
  }
  };
  
  } // namespace jge

So this is what I want, without the `//` comment:

  namespace jge
  {
  template 
  struct [[nodiscard]] size2d
  {
  template 
  friend constexpr auto
  operator*(const size2d& l, const units::quantity& r) noexcept(
  noexcept(l.w* r)) requires(requires { l.w* r; }) 
  {
  return size2d{l.w * r, l.h * r};
  }
  };
  
  } // namespace jge

For the constructor initializer list, this is what I have. Note the parentheses 
in the first requires-clause:

  namespace jge
  {
  template 
  class [[nodiscard]] plane
  {
  constexpr plane(const plane& other) noexcept(
  (detail::default_allocator_is_nothrow &&
   std::is_nothrow_copy_constructible_v)) requires(std::copyable)
: plane{to1d(other), other.sz.w}
  {
  }
  
  constexpr plane& operator=(const plane& other) noexcept(
  (std::is_nothrow_copy_constructible_v &&
   std::is_nothrow_copy_assignable_v)) requires std::copyable //
  {
  assign(to1d(other), other.sz.w);
  return *this;
  }
  };
  
  } // namespace jge

This is what I get without the parentheses. Note that all code after the 
constructor initializer list is further indented:

  namespace jge
  {
  template 
  class [[nodiscard]] plane
  {
  constexpr plane(const plane& other) noexcept(
  (detail::default_allocator_is_nothrow &&
   std::is_nothrow_copy_constructible_v)) requires std::copyable
: plane{to1d(other), other.sz.w}
{
}
  
constexpr plane& operator=(const plane& other) noexcept(
(std::is_nothrow_copy_constructible_v &&
 std::is_nothrow_copy_assignable_v)) requires std::copyable //
{
assign(to1d(other), other.sz.w);
return *this;
}
  };
  
  } // namespace jge

So this is what I want. Without extra parentheses, nor extra indentation:

  namespace jge
  {
  template 
  class [[nodiscard]] plane
  {
  constexpr plane(const plane& other) noexcept(
  (detail::default_allocator_is_nothrow &&
   std::is_nothrow_copy_constructible_v)) requires std::copyable
: plane{to1d(other), other.sz.w}
  {
  }
  
  constexpr plane& operator=(const plane& other) noexcept(
  (std::is_nothrow_copy_constructible_v &&
   std::is_nothrow_copy_assignable_v)) requires std::copyable //
  {
  assign(to1d(other), other.sz.w);
  return *this;
  }
  };
  
  } // namespace jge

All with the config file 
 linked 
above.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

In D79773#2124231 , @JohelEGP wrote:

>   constexpr plane(const plane& other) noexcept(
>   (detail::default_allocator_is_nothrow &&
>std::is_nothrow_copy_constructible_v)) requires(std::copyable)
> : plane{to1d(other), other.sz.w}
>   {
>   }
>
>
> Without the parentheses in trailing requires-clauses, all code following the 
> constructor initializer list is further indented.


Just so I'm clear for this and your Allman coment would you show me what you 
are seeing and what you expect to see? just so I understand.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:13540
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

curdeius wrote:
> Format: leading whitespace. Should it be in the same "group" as 
> `AlignConsecutiveMacros`?
I might be misunderstanding your comment, but the list should alphabetic but 
now I realize it should be after the `Allows`



Comment at: clang/unittests/Format/FormatTest.cpp:16745
+   "::std::is_copy_constructable and "
+   "::std::is_move_constructable and\n"
+   "requires (T c) {\n"

curdeius wrote:
> Isn't it a strange indentation?
there is no newline on the line above, but I clang-format the tests so it wraps 
it, I know visually its confusing


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

  constexpr plane(const plane& other) noexcept(
  (detail::default_allocator_is_nothrow &&
   std::is_nothrow_copy_constructible_v)) requires(std::copyable)
: plane{to1d(other), other.sz.w}
  {
  }

Without the parentheses in trailing requires-clauses, all code following the 
constructor initializer list is further indented.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/include/clang/Format/Format.h:533
 
+  /// If ``true``, always break before concept declarations
+  bool AlwaysBreakBeforeConceptDeclarations;

It would be nice to have an example here in the doc.



Comment at: clang/unittests/Format/FormatTest.cpp:13540
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

Format: leading whitespace. Should it be in the same "group" as 
`AlignConsecutiveMacros`?



Comment at: clang/unittests/Format/FormatTest.cpp:16745
+   "::std::is_copy_constructable and "
+   "::std::is_move_constructable and\n"
+   "requires (T c) {\n"

Isn't it a strange indentation?


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP requested changes to this revision.
JohelEGP added a comment.
This revision now requires changes to proceed.

`BreakBeforeBraces: Allman` isn't respected.

  template 
  friend constexpr auto
  operator*(const size2d& l, const units::quantity& r) noexcept(
  noexcept(l.w* r)) requires(requires { l.w* r; }) //
  {
  return size2d{l.w * r, l.h * r};
  }

When a trailing //requires-clause// is right before the function body, its 
opening brace should go on its own line, as dictated by my config file 
.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 265885.
MyDeveloperDay added a comment.

switch function to unsigned


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13537,6 +13537,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveBitFields);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -13557,6 +13558,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16567,6 +16569,223 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+ 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2296
+
+void UnwrappedLineParser::parseRequiresExpression(int OriginalLevel) {
+  // requires (R range)

Line->Level is an unsigned int so this gives me a warning. I guess it should be 
`unsigned int OriginalLevel` same below


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 265847.
MyDeveloperDay added a comment.

Rebase


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13537,6 +13537,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveBitFields);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -13557,6 +13558,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16567,6 +16569,222 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "requires Foo() && Bar {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "requires 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D79773#2046833 , @miscco wrote:

> It seems the spacing of the binary operator is not yet stable. This test is 
> breaking for me:
>
>   verifyFormat("template \nconcept someConcept = Constraint1 
> && Constraint2;");
>


Hmm... are you upto date

  $ ./FormatTests.exe --gtest_filter=*Concept*
  Note: Google Test filter = *Concept*
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
  [--] 1 test from FormatTest
  [ RUN  ] FormatTest.ConceptsAndRequires
  [   OK ] FormatTest.ConceptsAndRequires (642 ms)
  [--] 1 test from FormatTest (645 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test case ran. (650 ms total)
  [  PASSED  ] 1 test.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

It seems the spacing of the binary operator is not yet stable. This test is 
breaking for me:

  verifyFormat("template \nconcept someConcept = Constraint1 && 
Constraint2;");


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

In D79773#2046565 , @miscco wrote:

> Question: Should I add my wip work as a child revision or what would you 
> suggest


Certainly your revision on github was useful for me to take a look at, but to 
be honest this change is getting large and I don't like landing such a large 
change in one go but rather make small incremental improvements (I believe this 
is the LLVM way)

Feel free to create a revision (and reference it here). Once we land some form 
of initial implementation I feel we can start making improvement as we detect 
corner cases or adding new options (and that might be more easily spread 
between developers)

To be honest I would rather start getting bugs from external users than 
clang-format being the butt of peoples complaints 
https://twitter.com/the_moisrex/status/1262982971973423105


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

Question: Should I add my wip work as a child revision or what would you suggest


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 7 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2281
+  if (FormatTok->Tok.is(tok::kw_requires))
+parseRequires();
+}

miscco wrote:
> I believe this should be `parseConstraintExpression`  because that is the 
> term of art in the standard.
> 
> The requires expression is what is the `requieres { }` and that can be part 
> of an constraint expression 
I'm making some of these changes, the problem is with the form

`concept id = Foo && Bar`

in this case the 

`Foo && Bar` 

will be processed by the `parseStructualElement()` and this gets confused with 
the && being a `TT_PointerOrRefernce` and not a `TT_BinaryOperator` and that 
throws everything out.

I think parseStructualElement() is thinking `Foo &&` is part of

`foo(Foo &);`

and not

`Foo && abc`

We need to know we are in a concept expression





Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

miscco wrote:
> miscco wrote:
> > I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> > /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
> To be more specific, I am concerned what happens if you have multiple 
> template arguments where a linebreak should occur. Is this still happening 
> here?
> 
> 
> ```
> template 
> concept someVeryLongConceptName = 
> someVeryLongConstraintName1;
> ```
This is formatted as

```
template 
concept someVeryLongConceptName =
someVeryLongConstraintName1;
```


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 265216.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added reviewers: miscco, JakeMerdichAMD.
MyDeveloperDay added a comment.

Add new `AlwaysBreakBeforeConceptDeclarations`

Better handling of requires expressions vs constraint expression

add more concept tests from the twitter-sphere


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13500,6 +13500,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveBitFields);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -13520,6 +13521,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16466,6 +16468,219 @@
Style);
 }
 
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay planned changes to this revision.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1566
   Current.Type = TT_TrailingReturnArrow;
-
+} else if (Current.is(tok::arrow) && Current.Previous &&
+   Current.Previous->is(tok::r_brace)) {

miscco wrote:
> MyDeveloperDay wrote:
> > miscco wrote:
> > > Should that really be `tok::arrow` and not `tok::kw_requires`?
> > This is to prevent the breaking between } and -> in the following (which 
> > could be some way from the requires
> > 
> > `{ t.foo(u) } -> typename T::foo_type;`
> I believe this should carry some state that we are inside of an 
> requires-expression. 
I'm not sure how possible that is as its dealt with in terms of normal 
parseBlock() handling so there might not be a possiblity to label the -> (which 
is actually what I'm just doing here) the other rules regarding 
TT_TrailingReturnArrow will ensure its doesn't get broken and there is spaces 
around it.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

miscco wrote:
> I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
To be more specific, I am concerned what happens if you have multiple template 
arguments where a linebreak should occur. Is this still happening here?


```
template 
concept someVeryLongConceptName = someVeryLongConstraintName1;
```



Comment at: clang/unittests/Format/FormatTest.cpp:16319
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"

I think `concept bool` should just be `concept` occurs below 


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
/*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2281
+  if (FormatTok->Tok.is(tok::kw_requires))
+parseRequires();
+}

I believe this should be `parseConstraintExpression`  because that is the term 
of art in the standard.

The requires expression is what is the `requieres { }` and that can be part of 
an constraint expression 


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

You are missing to check the boolean in `CHECK_PARSE_BOOL` in FormatTest.cpp




Comment at: clang/lib/Format/TokenAnnotator.cpp:1566
   Current.Type = TT_TrailingReturnArrow;
-
+} else if (Current.is(tok::arrow) && Current.Previous &&
+   Current.Previous->is(tok::r_brace)) {

MyDeveloperDay wrote:
> miscco wrote:
> > Should that really be `tok::arrow` and not `tok::kw_requires`?
> This is to prevent the breaking between } and -> in the following (which 
> could be some way from the requires
> 
> `{ t.foo(u) } -> typename T::foo_type;`
I believe this should carry some state that we are inside of an 
requires-expression. 



Comment at: clang/lib/Format/TokenAnnotator.cpp:3484
+  // concept ...
+  if (Left.is(TT_TemplateCloser) && Right.is(tok::kw_concept))
+return true;

MyDeveloperDay wrote:
> miscco wrote:
> > I think we should have an option to allow short requires clauses on the 
> > same line similar to allow short functions
> Actually at present it will pull short requires onto a new line, this 
> particular line is about keeping `concept` on a new line (most examplesI've 
> seen are like this..i.e.
> 
> ```template
> concept...```
> 
> rather than 
> 
> `template concept...`
> 
> For me the natural thing here is to enforce a newline and then relax that 
> later if we think it needs it, but I'm wary of adding too many options 
> upfront. 
> 
> This is virgin territory as most style guides out there don't specify 
> anything for concepts yet, to be honest i'd prefer to establish an LLVM style 
> and wait for the other style guides to catch up. (I know we will revisit this 
> later), just want to keep the initial commit small. (incremental development 
> is encouraged in LLVM) and multiple options will be a huge commit that never 
> gets passed.
> 
> 
Personally I agree that this is the natural style, at the same time there are 
configurations that allow short template declarations on a single line, which I 
believe would be similarly welcome. 


That said I agree adding this later is fine



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2284
+
+void UnwrappedLineParser::parseRequires() {
+  assert(FormatTok->Tok.is(tok::kw_requires) && "'requires' expected");

MyDeveloperDay wrote:
> miscco wrote:
> > I believe we should separate between constraint expressions aka: `tempalte< 
> >  > requires Concept1 && Concept2` and requires expressions aka `requires { 
> > ... }`
> > 
> > The two are quite different. That said the handling of requires expressions 
> > should be "simple" as we only need to find the matching "}". 
> > 
> > As an upside this would also handle the famous `requires requires`
> This is effectively the `if` at line 2305 seeing "requires {" or requires(Foo 
> t) {" sets off down the parseBlock path which can itself include requires.
> 
> If you do have an example you think would break this please feel free to add 
> that in a comment and I'll add it to the unit tests and rework accordingly.
> 
> My exposure to concepts is still pretty new, I'm not even sure I've covered 
> all the places it can be used, but I wanted to start because at present I see 
> alot of the MS STL(@STL_MSFT ) covered with // clang-format off around the 
> concepts.
Yes that was also the reason I tried it out because I often added those // 
clang-format off snippets once too often 


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 264851.
MyDeveloperDay added a comment.

Move break before concept into correct clause
Add `requires requires` test


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16306,6 +16306,145 @@
  Style));
 }
 
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  Style.IndentRequires = true;
+  verifyFormat("template \n"
+   "  requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "  requires(index_ < sizeof...(Children_))\n"
+   "Tree auto () {\n"
+   "  // ...\n"
+   "}",
+   Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("template \n"
+   "concept Hashable = requires (T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "  requires EqualityComparable || Same\n"
+   "struct equal_to;",
+   Style);
+
+  verifyFormat("template \n"
+  

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay planned changes to this revision.
MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3499
 return true;
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&

miscco wrote:
> I think that your change should actually come in here where we determine what 
> to do after a `ClosesTemplateDeclaration` 
> 
> With an potential new option `AlwaysBreakConceptDeclarations ` that should 
> probably default to `AlwaysBreakTemplateDeclarations ` this would read
> ```
>   if (Right.Previous->ClosesTemplateDeclaration &&
>   Right.Previous->MatchingParen &&
>   Right.Previous->MatchingParen->NestingLevel == 0) { 
>   if (Right.is(tok::kw_requires)) {
> switch(Style.AllowShortRequiresClause) {
>   case FormatStyle::SRCS_Never: 
> return true;
>   case FormatStyle::SRCS_Always: 
> return false;
>   case FormatStyle::SRCS_Single:
> // TODO: Determine whether there is a single constraint 
> return true;
>   case FormatStyle::SRCS_Short: 
> // TODO: Determine whether the constraint clause is short enough
> return true;
> } 
>   } else if (Right.is(tok::kw_concept)) {
> return Style.AlwaysBreakConceptDeclarations == FormatStyle::BTCS_Yes);
>   } else {
> return Style.AlwaysBreakTemplateDeclarations == 
> FormatStyle::BTDS_Yes);
>   }
>   }
> ```
yes this is a better place


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3499
 return true;
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&

I think that your change should actually come in here where we determine what 
to do after a `ClosesTemplateDeclaration` 

With an potential new option `AlwaysBreakConceptDeclarations ` that should 
probably default to `AlwaysBreakTemplateDeclarations ` this would read
```
  if (Right.Previous->ClosesTemplateDeclaration &&
  Right.Previous->MatchingParen &&
  Right.Previous->MatchingParen->NestingLevel == 0) { 
  if (Right.is(tok::kw_requires)) {
switch(Style.AllowShortRequiresClause) {
  case FormatStyle::SRCS_Never: 
return true;
  case FormatStyle::SRCS_Always: 
return false;
  case FormatStyle::SRCS_Single:
// TODO: Determine whether there is a single constraint 
return true;
  case FormatStyle::SRCS_Short: 
// TODO: Determine whether the constraint clause is short enough
return true;
} 
  } else if (Right.is(tok::kw_concept)) {
return Style.AlwaysBreakConceptDeclarations == FormatStyle::BTCS_Yes);
  } else {
return Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes);
  }
  }
```


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added a subscriber: STL_MSFT.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1566
   Current.Type = TT_TrailingReturnArrow;
-
+} else if (Current.is(tok::arrow) && Current.Previous &&
+   Current.Previous->is(tok::r_brace)) {

miscco wrote:
> Should that really be `tok::arrow` and not `tok::kw_requires`?
This is to prevent the breaking between } and -> in the following (which could 
be some way from the requires

`{ t.foo(u) } -> typename T::foo_type;`



Comment at: clang/lib/Format/TokenAnnotator.cpp:3484
+  // concept ...
+  if (Left.is(TT_TemplateCloser) && Right.is(tok::kw_concept))
+return true;

miscco wrote:
> I think we should have an option to allow short requires clauses on the same 
> line similar to allow short functions
Actually at present it will pull short requires onto a new line, this 
particular line is about keeping `concept` on a new line (most examplesI've 
seen are like this..i.e.

```template
concept...```

rather than 

`template concept...`

For me the natural thing here is to enforce a newline and then relax that later 
if we think it needs it, but I'm wary of adding too many options upfront. 

This is virgin territory as most style guides out there don't specify anything 
for concepts yet, to be honest i'd prefer to establish an LLVM style and wait 
for the other style guides to catch up. (I know we will revisit this later), 
just want to keep the initial commit small. (incremental development is 
encouraged in LLVM) and multiple options will be a huge commit that never gets 
passed.





Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2284
+
+void UnwrappedLineParser::parseRequires() {
+  assert(FormatTok->Tok.is(tok::kw_requires) && "'requires' expected");

miscco wrote:
> I believe we should separate between constraint expressions aka: `tempalte<  
> > requires Concept1 && Concept2` and requires expressions aka `requires { ... 
> }`
> 
> The two are quite different. That said the handling of requires expressions 
> should be "simple" as we only need to find the matching "}". 
> 
> As an upside this would also handle the famous `requires requires`
This is effectively the `if` at line 2305 seeing "requires {" or requires(Foo 
t) {" sets off down the parseBlock path which can itself include requires.

If you do have an example you think would break this please feel free to add 
that in a comment and I'll add it to the unit tests and rework accordingly.

My exposure to concepts is still pretty new, I'm not even sure I've covered all 
the places it can be used, but I wanted to start because at present I see alot 
of the MS STL(@STL_MSFT ) covered with // clang-format off around the concepts.


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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

Awesome, Thank you very much, I dragged my feet on starting to implement 
something for real.

As a high level comment I think we need to handle requires expressions to get 
this correct, as `requires requires` would otherwise to bad things.

From my early brainstorming i got the following options I thought useful. This 
is in no ways a request for you to implement them, but I would want to get them 
out here in case they are usefull

  /// Different styles for merging short requires-clauses with the template
/// decalaration
enum ShortRequiresClauseStyle {
  /// Never merge requires clauses into a single line.
  /// \code
  ///   template 
  ///   requires someConstraint
  ///   T foo();
  /// \endcode
  SRCS_Never,
  /// Only merge requires clauses with a single constraint.
  /// \code
  ///   template  requires someConstraint
  ///   T foo();
  ///
  ///   template 
  ///   requires someConstraint && otherConstraint
  ///   T bar();
  /// \endcode
  SRCS_Single,
  /// Merge requires clauses if they are short
  /// \code
  ///   template  requires short && shorter
  ///   T foo();
  ///
  ///   template 
  ///   requires someReallyLongElaborateConstraintThatIsReallyLong
  ///   T bar();
  ///
  ///   template 
  ///   requires someLongConstraint && otherLongConstraint
  ///   T baz();
  /// \endcode
  SRCS_Short,
  /// Always try to merge the requires clause with the template declaration
  /// \code
  ///   template  requires someLongConstraint &&
  ///   otherLongConstraint
  ///   T foo();
  /// \endcode
  SRCS_Always,
};
  
/// Dependent on the value, requires-clauses can be put on the same line
/// as the template declaration.
ShortRequiresClauseStyle AllowShortRequiresClause;
  
/// Dependent on the value, requires-expressions can be a single line
/// Always try to merge the requires clause with the template declaration
/// \code
///   template  requires someLongConstraint &&
///   requires { T{}; }
///   T foo();
/// \endcode
bool AllowShortRequiresExpression;
  
 /// Dependent on the value break before or after constraint expressions.
enum ConstraintExpressionBreakingStyle {
  /// Break after constraint expressions but multiple may be on single line
  /// \code
  ///   template  requires someLongConstraint &&
  ///   otherLongConstraint
  ///   T foo();
  ///
  ///   template  requires short && shorter &&
  ///   otherLongConstraint
  ///   T bar();
  /// \endcode
  CEBS_After,
  /// Break after constraint expressions.
  /// \code
  ///   template  requires short &&
  ///   shorter &&
  ///   otherLongConstraint
  ///   T bar();
  /// \endcode
  CEBS_AfterSingleExpression,
  /// Break before constraint expressions but multiple may be on single line
  /// \code
  ///   template  requires someLongConstraint &&
  ///   otherLongConstraint
  ///   T foo();
  ///
  ///   template  requires short && shorter
  ///&& otherLongConstraint
  ///   T bar();
  /// \endcode
  CEBS_Before,
  /// Break before constraint expressions.
  /// \code
  ///   template  requires short
  ///&& shorter
  ///&& otherLongConstraint
  ///   T foo();
  /// \endcode
  CEBS_BeforeSingleExpression,
};
  
/// The constraint expressions style to use.
ConstraintExpressionBreakingStyle BreakBeforeConstraintExpression;
  
/// Dependent on the value wrap before or after requires expressions.
enum BraceWrappingRequiresExpressionStyle {
  /// Do not wrap braces of requires expressions
  /// \code
  ///   template  requires requires {  T{};
  ///   T(int); }
  ///   T foo();
  /// \endcode
  BWARES_NoWrap,
  /// Wrap after requires expressions.
  /// \code
  ///   template  requires requires { T{};
  ///  T(int);
  ///}
  ///   T foo();
  /// \endcode
  BWARES_WrapAfter,
  /// Wrap after requires expressions.
  /// \code
  ///   template  requires requires
  ///   { T{};
  /// T(int); }
  ///   T foo();
  /// \endcode
  BWARES_WrapBefore,
  /// Wrap after requires expressions with a new line.
  /// \code
  ///   template  requires requires
  ///   

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 263494.
MyDeveloperDay added a reviewer: saar.raz.
MyDeveloperDay added a subscriber: saar.raz.
MyDeveloperDay added a comment.

Add additional parsing of concept and require keywords to allow formatting

Include some more of @saar.raz examples in the tests


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

https://reviews.llvm.org/D79773

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.cpp.new
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15849,6 +15849,51 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, ConceptsImplicitConversionConstraint) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};",
+   Style);
+}
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -113,6 +113,8 @@
   void parseNew();
   void parseAccessSpecifier();
   bool parseEnum();
+  void parseConcept();
+  void parseRequires();
   void parseJavaEnumBody();
   // Parses a record (aka class) as a top level element. If ParseAsExpr is true,
   // parses the record as a child block, i.e. if the class declaration is an
Index: clang/lib/Format/UnwrappedLineParser.cpp.new
===
--- /dev/null
+++ clang/lib/Format/UnwrappedLineParser.cpp.new
@@ -0,0 +1,3002 @@
+//===--- UnwrappedLineParser.cpp - Format C++ code ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file contains the implementation of the UnwrappedLineParser,
+/// which turns a stream of tokens into UnwrappedLines.
+///
+//===--===//
+
+#include "UnwrappedLineParser.h"
+#include "FormatToken.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+
+#define DEBUG_TYPE "format-parser"
+
+namespace clang {
+namespace format {
+
+class FormatTokenSource {
+public:
+  virtual ~FormatTokenSource() {}
+  virtual FormatToken *getNextToken() = 0;
+
+  virtual unsigned getPosition() = 0;
+  virtual FormatToken *setPosition(unsigned Position) = 0;
+};
+
+namespace {
+
+class ScopedDeclarationState {
+public:
+  ScopedDeclarationState(UnwrappedLine , std::vector ,
+ bool MustBeDeclaration)
+  : Line(Line), Stack(Stack) {
+Line.MustBeDeclaration = MustBeDeclaration;
+Stack.push_back(MustBeDeclaration);
+  }
+  ~ScopedDeclarationState() {
+Stack.pop_back();
+if (!Stack.empty())
+  Line.MustBeDeclaration = Stack.back();
+else
+  Line.MustBeDeclaration = true;
+  }
+
+private:
+  UnwrappedLine 
+  std::vector 
+};
+
+static bool isLineComment(const FormatToken ) {
+  return FormatTok.is(tok::comment) && !FormatTok.TokenText.startswith("/*");
+}
+
+// Checks if 

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1924
 return TT_BinaryOperator;
 }
 

This change it to resolve the missing gap between `&&` and `Concept2`, 
clang-format incorrectly labels the && as TT_PointerOrReference

```
template  void f(I i) requires Concept1 & {
  // ...
}
```

vs

```
template  void f(I i) requires Concept1 && Concept2 {
  // ...
}
```



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

https://reviews.llvm.org/D79773



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, sammccall, owenpan, jbcoe, 
mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.

This is a starting point to improve the handling of concepts in clang-format. 
There is currently no real formatting of concepts and this can lead to some odd 
formatting, e.g.

  requires(R range) {
typename Iterator_type;
{ begin(range) }
->Iterator_type;
{ end(range) }
->Iterator_type;
requires Input_iterator>();
  };

The revision starts by resolving the additional newline added before the 
implicit conversion constraint and ensures that the concept keyword is always 
on the line below template<>

  template 
  concept bool EqualityComparable = requires(T a, T b) {
{ a == b } -> bool;
  };




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79773

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15849,6 +15849,41 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, ConceptsImplicitConversionConstraint) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("requires(R range) {\n"
+   "  typename Iterator_type;\n"
+   "  { begin(range) } -> Iterator_type;\n"
+   "  { end(range) } -> Iterator_type;\n"
+   "  requires Input_iterator>();\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -628,6 +628,13 @@
 
   if (MunchSemi && FormatTok->Tok.is(tok::semi))
 nextToken();
+  else if (FormatTok->is(tok::arrow)) {
+// Following the } we can find a trailing return type arrow
+// as part of an implicit conversion constraint.
+nextToken();
+parseStructuralElement();
+  }
+
   Line->Level = InitialLevel;
 
   if (PPStartHash == PPEndHash) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1563,7 +1563,11 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
-
+} else if (Current.is(tok::arrow) && Current.Previous &&
+   Current.Previous->is(tok::r_brace)) {
+  // Concept implicit conversion contrain needs to be treated like
+  // a trailing return type  ... } -> .
+  Current.Type = TT_TrailingReturnArrow;
 } else if (isDeductionGuide(Current)) {
   // Deduction guides trailing arrow " A(...) -> A;".
   Current.Type = TT_TrailingReturnArrow;
@@ -3466,6 +3470,12 @@
   return true;
   }
 
+  // Put concepts on the next line e.g.
+  // template
+  // concept ...
+  if (Left.is(TT_TemplateCloser) && Right.is(tok::kw_concept))
+return true;
+
   if (Right.is(tok::comment))
 return Left.BlockKind != BK_BracedInit &&
Left.isNot(TT_CtorInitializerColon) &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15849,6 +15849,41 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, ConceptsImplicitConversionConstraint) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept bool