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

STL: Cleanup concept definitions #570

Closed
CaseyCarter opened this issue Feb 28, 2020 · 4 comments
Closed

STL: Cleanup concept definitions #570

CaseyCarter opened this issue Feb 28, 2020 · 4 comments
Labels
enhancement Something can be improved resolved Successfully resolved without a commit

Comments

@CaseyCarter
Copy link
Member

Per discussion at #565 (review).

Generally I believe that there should be a bigger focus on readability of the concepts. Currently a lot of the more complex concepts are really hard to grog due to unfortunate formatting. I believe that we would gain a lot by spending a few lines more to improve the general readability.

I agree. I've been trying to format the constraint-expressions in concepts as clang-format would - with the expectation that it will reduce churn someday when clang-format learns to understand concepts and requires-clauses - but I've started cheating here and there when it helps readability. We should simply go all in and format them one-term-per-line (except for very simple cases) as in the Ranges TS.

I'll update the concept definitions that this PR touches, and file an issue to make a general audit.

We should audit all concept definitions for readability. Some guidelines:

  • The first line of the definition may contain multiple terms if they aren't too complex visually.
  • Try not to split disjunctions across multiple lines if possible (we prefer Conjunctive Normal Form)
  • Continuation lines should generally have a single term, although there are exceptional cases where multiple terms can better convey structure. For example:
    template <class _Ty>
    concept copy_constructible = move_constructible<_Ty>
        && constructible_from<_Ty, _Ty&>       && convertible_to<_Ty&, _Ty>
        && constructible_from<_Ty, const _Ty&> && convertible_to<const _Ty&, _Ty>
        && constructible_from<_Ty, const _Ty>  && convertible_to<const _Ty, _Ty>;
  • Terms with many and/or complex arguments can be split across multiple lines:
    template <class _Fn, class _It>
    concept indirectly_unary_invocable = indirectly_readable<_It>
        // ...snip...
        && common_reference_with<
            invoke_result_t<_Fn&, iter_value_t<_It>&>,
            invoke_result_t<_Fn&, iter_reference_t<_It>>>;
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Feb 28, 2020
@StephanTLavavej
Copy link
Member

Have issues been filed against clang-format so it will format concepts better someday? I would like to avoid too much manual formatting in the codebase.

@CaseyCarter
Copy link
Member Author

There are at least LLVM-32165 and LLVM-32166 (which I've just poked).

@frederick-vs-ja
Copy link
Contributor

By #4947 we're now formatting concept definitions by Clang-format (except for common_with). Should this considered resolved?

(Although I wonder if it's possible to make Clang-format accept Cat() requires default_initializable<Meow> = default; in a single line.)

@CaseyCarter
Copy link
Member Author

I agree this issue is outdated.

@CaseyCarter CaseyCarter added the resolved Successfully resolved without a commit label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved resolved Successfully resolved without a commit
Projects
None yet
Development

No branches or pull requests

3 participants