Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

always parenthesize the constraint-logical-or-expression in a requires-clause #2504

Open
zygoloid opened this issue Nov 22, 2018 · 4 comments
Open

always parenthesize the constraint-logical-or-expression in a requires-clause #2504

zygoloid opened this issue Nov 22, 2018 · 4 comments

Comments

@zygoloid
Copy link
Member

zygoloid commented Nov 22, 2018 •
edited

We're starting to add requires-clauses to standard library functions, and they can quickly become pretty unreadable. For example, the Ranges proposal adds:

constexpr pointer operator->() const requires is_pointer_v<Iterator> ||
  requires (const Iterator i) { i.operator->(); };

... which makes it look like there are multiple requires-clauses or something. Add to that the fact that some requires-clauses require parentheses anyway, and the weirdness of things like

void f() noexcept(blah) requires see below;

(How much will I see below? Just a constraint, or other things that might go there, such as a function body or so?) ... and I think the only sensible and consistent formatting decision is to say that we always parenthesize the condition in a requires-clause. We should document this decision in the Wiki and apply it throughout the draft after merging the 2018-11 motions.
@zygoloid zygoloid added the after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied. label Nov 22, 2018
@zygoloid zygoloid added this to the post-2018-11 milestone Nov 22, 2018
@jensmaurer
Copy link
Member

In the interest of clarity, for your two examples, you'd want

constexpr pointer operator->() const requires (is_pointer_v<Iterator> ||
  requires (const Iterator i) { i.operator->(); });

(with some indentation / line wrapping fixes) and

void f() noexcept(blah) requires (_see below_);

?

@zygoloid
Copy link
Member Author

Line-wrapping for the first one should be

constexpr pointer operator->() const
  requires (is_pointer_v<Iterator> ||
            requires (const Iterator i) { i.operator->(); });

or similar, but yes, that's what I'm looking for.

@jensmaurer
Copy link
Member

jensmaurer commented Nov 26, 2018 •
edited

If we do the indentation right, it seems to me that "always-parens" is a bit over the top. Maybe putting parens around the requires-expression is good enough; we should not have too many _requires-expression_s to start with.

@jensmaurer jensmaurer removed this from the post-2018-11 milestone Nov 27, 2018
@jensmaurer jensmaurer added decision-required A decision of the editorial group (or the Project Editor) is required. and removed after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied. labels Nov 27, 2018
@jensmaurer
Copy link
Member

Editorial meeting:

  • Sympathetic to parenthesizing the entire constraint-logical-or-expression in the requires-clause.
  • Do not act for now.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zygoloid @jensmaurer