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

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 ma

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

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

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

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

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

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 LL

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

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 ___

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

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/m

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

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-

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

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 nee

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

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

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

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

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

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:/

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

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)) { kli

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

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/Cl

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

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 somet

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

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 p

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

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 m

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

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/c

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

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/Release

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

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

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

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 requi

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

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(); +parseS

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

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

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

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

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

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 EqualityCo

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

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 li

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

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 bef

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

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_

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

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 = requir

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

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`

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

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(AllowAllArgum

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

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(AllowAllArgumentsOnNextLi

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

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/ClangFormatStyleOption

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

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 SIN

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

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

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

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

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

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_construct

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

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(AllowAllArgum

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

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 cod

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

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. ===

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

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(

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

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 clan

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

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

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

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/Forma

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

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;"); >

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

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/D7977

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

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

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

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

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

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 `parseCo

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

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 expres

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

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) && C

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

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(/*Con

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

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(/*ContinueOnSemicolon

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

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. T

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

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 && +

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

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/ClangFormatStyleO

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

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

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

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 determi

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

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) && Curre

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

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 g

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

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 te

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

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-f

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

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