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.

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

[feature request] requires clause in class templates (Concepts TS) #31513

Closed
gnzlbg mannequin opened this issue Mar 7, 2017 · 8 comments
Closed

[feature request] requires clause in class templates (Concepts TS) #31513

gnzlbg mannequin opened this issue Mar 7, 2017 · 8 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang-format confirmed Verified by a second party

Comments

@gnzlbg
Copy link
Mannequin

gnzlbg mannequin commented Mar 7, 2017

Bugzilla Link 32165
Version trunk
OS All
CC @CaseyCarter,@HazardyKnusperkeks,@Sebanisu

Extended Description

The following code:

template <class T, class U, class... Rest>
requires CommonReference<T, U>()
struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

is reformatted to

template <class T, class U, class... Rest>
requires CommonReference<T, U>() struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

because clang-format does not understand the requires keyword in class templates.

It should either be formatted to

// A: requires clause in its own line:
template <class T, class U, class... Rest>
requires CommonReference<T, U>()
struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

// B: small require clause in line with template arguments
template <class T, class U, class... Rest> requires CommonReference<T, U>()
struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

Also, long requires clauses are not handled properly

template <class T, class U, class... Rest>
requires CommonReference<T, U>() &&
CommonReference<T, U>() &&
CommonReference<T, U>()
struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

is reformatted to:

template <class T, class U, class... Rest>
requires CommonReference<T, U>() && CommonReference<T, U>() &&
CommonReference<T, U>() struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

Following customization parameters might make sense, but I haven't given this much thought:

  • indentation of the require clause with respect to the template arguments/struct (e.g. 1 space, 4 spaces...). Maybe we could reuse ContinuationIndentWidth/AccessModifierOffset for this, but I'd rather have it as a standalone option.
  • AllowShortRequireClausesOnASingleLine: whether short requires clauses should be wrapped into the same line as the template arguments

And well probably stuff like BreakBeforeBinaryOperators should work here properly.

@gnzlbg
Copy link
Mannequin Author

gnzlbg mannequin commented Mar 7, 2017

assigned to @HazardyKnusperkeks

@gnzlbg
Copy link
Mannequin Author

gnzlbg mannequin commented Mar 7, 2017

The STL2 formats requires clauses in some places like this:

template <class T, class U, class... Rest>
requires
CommonReference<T, U>() &&
CommonReference<T, U>() &&
CommonReference<T, U>()
struct common_reference<T, U, Rest...>
: common_reference<common_reference_t<T, U>, Rest...> {};

That is:

  • RequiresClauseOffset = 0 (requires starts at the same place as template and struct)
  • the first elements is put into the next line (there will need to be an option just for this).

https://github.com/CaseyCarter/cmcstl2/blob/49a0a43641f34ba1eb8170a1670a00ee3c0a8975/include/stl2/view/indirect.hpp#L24

@CaseyCarter
Copy link
Member

Poke, poke. Now that C++20 has been sent out for (what will likely be) its final round of balloting, it would be a good time to work on supporting requires-clauses in clang-format. Think of all the poor, suffering STL maintainers!

@Sebanisu
Copy link

Sebanisu commented Nov 4, 2021

In version 13 they have an IndentRequires that basically either indents or not.

@HazardyKnusperkeks
Copy link
Contributor

I'm working on it.

@Sebanisu
Copy link

mentioned in issue llvm/llvm-bugzilla-archive#52401

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@HazardyKnusperkeks HazardyKnusperkeks added the awaiting-review Has pending Phabricator review label Feb 5, 2022
@HazardyKnusperkeks
Copy link
Contributor

Review link: https://reviews.llvm.org/D113319

@HazardyKnusperkeks
Copy link
Contributor

Pushed

@github-actions github-actions bot removed the awaiting-review Has pending Phabricator review label Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-format confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

4 participants