[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

Emilia Dreamer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 11 00:11:49 PST 2023


rymiel added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:809
                    Style.AllowShortCaseLabelsOnASingleLine);
+    IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+                   Style.AllowShortCompoundRequirementOnASingleLine);
----------------
Backl1ght wrote:
> MyDeveloperDay wrote:
> > haven't we use "Requires" in other options? What is the definition of Compound?
> > 
> > I might be tempted for this to be AllShortRequiresOnASingleLine but then it be an enum with the following options
> > 
> > ```
> > Leave
> > Never
> > Always
> > Compound
> > ```
> There are some options related to requires like IndentRequiresClause, RequiresClausePosition, etc. But as for single line and brace wrapping, requires is not yet considered.
> 
> "Compound" is from Compound Requirements section in this page https://en.cppreference.com/w/cpp/language/requires
> 
> I think this is a better way, I would try implementing it.
A //compound-requirement// is what the standard calls it, so I think the name is fine.

I'm not sure if collating it with other options as suggested is a good idea, since a //requires-expression// can contain multiple //compound-requirement//s, such as:

```
requires {
  { E1 } -> C;
  { E2 } -> D<A1>;
};
```

This //requires-expression// has two //compound-requirement//s, and two formatting of those two individually is what's being controlled by the option.

Although I will agree it should probably still be an enum, but in that case, it probably shouldn't start with "Allow"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834



More information about the cfe-commits mailing list