Quuxplusone added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3756
+ * ``bool AfterRequiresClause`` If ``true``, put space between requires
keyword in a requires clause and
+ opening parentheses, if is are one.
+
----------------
HazardyKnusperkeks wrote:
> curdeius wrote:
> > You meant "if there is one", right?
> Yeah
IMO these options should be named `InRequiresClause` and
`InRequiresExpression`: that's where the space is going. The space doesn't go
//after// the requires-clause. The space doesn't go //after// the
requires-expression.
It occurs to me that the name of this option could be analogous to the name of
the option that controls `[]() {}` versus `[] () {}`... except that it looks
like there is no such option (and I'm happy about that). :) Also, the name of
that option would probably just be `AfterCaptureList`, which doesn't help us in
this case.
================
Comment at: clang/include/clang/Format/Format.h:3371
+ /// If ``true``, put space between requires keyword in a requires clause
and
+ /// opening parentheses, if there is one.
+ /// \code
----------------
Here and line 3380, and possibly elsewhere: `s/parentheses/parenthesis/`
================
Comment at: clang/unittests/Format/FormatTest.cpp:14369
+ "{}",
+ SpaceAfterRequires);
}
----------------
Throughout, I'd like to see simpler test cases and more of them. E.g. I think
this would suffice, in lieu of lines 14305–14369:
```
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires (requires (int i) { x+i; }) { }",
SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires (requires(int i) { x+i; }) { }",
SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires(requires (int i) { x+i; }) { }",
SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires(requires(int i) { x+i; }) { }",
SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113369/new/
https://reviews.llvm.org/D113369
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
