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<typename T>
> concept...```
>
> rather than
>
> `template<typename T> 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
