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

feat: recognize requires expressions #596

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Aug 17, 2023

This PR adds support for requires-expressions (first based on #588 (comment)).

This is the syntax, based on Cpp1's:

Cpp2 Cpp1
number: <T> concept = requires(c: T) { template<typename T> concept number = requires(T const& c) {
number_difference_t<T>; typename number_difference_t<T>;
_ = T(); T();
{ c - c } !throws; { c - c } noexcept;
{ c + c } is std::common_with<T>; { c + c } -> std::common_with<T>;
requires std::common_with<number_difference_t<T>, T>; requires std::common_with<number_difference_t<T>,T>;
!requires std::iter_reference_t<T>; requires !requires { typename std::iter_reference_t<T>; };
}; };

The feature, as added by this PR, prevents or completely avoids known Cpp1 pitfalls.
By requiring that expression requirements are discarded, it should help to prevent the list of pitfalls at #588 (comment).
It also adds negative requirements as a first-class feature to create a pit of success for https://quuxplusone.github.io/blog/2021/06/09/another-concepts-chest-mimic/.

Additionally, I think we could:

@AbhinavK00
Copy link

AbhinavK00 commented Aug 17, 2023

( _ = T() );
{ _ = c - c } throws;

I find it weird that these use different kind of brackets. Since, compound expressions are just simple expressions with throws or is-type-requirement, they should look similar

@JohelEGP

This comment was marked as outdated.

@JohelEGP

This comment was marked as outdated.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Aug 17, 2023

To simplify that, in addition to

Additionally, I think we could:

  • Make negative-requirement a primary expression to simplify if constexpr (!requires { (t.begin()); }) to if constexpr (!requires t.begin();).

we could make a requires-expression's body also be a single requirement.

    //G requires-expression:
    //G     'requires' parameter-declaration-list? requirement-body
    //G     '!'? 'requires' parameter-declaration-list? requirement
requires requires(c: T) { _ = c - c; } // Long-winded spelling.
requires requires(c: T) _ = c - c;     // Simpler spelling.
requires !requires std::iter_reference_t<T>;      // OK.

To compare:

requires requires(const T& c) { c - c; } // Cpp1
requires requires(c: T) _ = c - c;     // Cpp2
requires requires(c: T) { _ = c - c; } // Cpp2
requires !requires { typename std::iter_reference_t<T>; } // Cpp1
requires !requires std::iter_reference_t<T>; // Cpp2

@AbhinavK00
Copy link

Make negative-requirement a primary expression to simplify if constexpr (!requires { (t.begin()); }) to if constexpr (!requires t.begin();)

This makes distinguishing between requires clauses and expressions even harder. In cpp1, I know that if there's no braces after requires, it will be a requires clause but if you allow this, then it can be requires clause or a requires expression in cpp2.

I would not sacrifice readability for one less pair of braces.

@JohelEGP
Copy link
Contributor Author

A requires clause can only happen before the = of a declaration.

@AbhinavK00
Copy link

Yes, but it still can be confusing to the reader.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Aug 17, 2023

I will doubt that until I find out code that makes me think so.
To me, the context dependence makes this pretty unambiguous.

@AbhinavK00
Copy link

Fair enough 👌

@JohelEGP
Copy link
Contributor Author

I'm changing this #526's and #596's use of throws.
The default will be the same as Cpp1's,
which is the same as most authored functions.
Instead, I'll extend throws-specifier to accept !throws,
which is a shorthand for https://wg21.link/P0709's throws(false).

With that change, you won't need to add throws
when rewriting Cpp1 non-noexcept function types or expression requirements for Cpp2.
For #596, I'll be able to re-add expression-requirement as _ = 𝘌;.
Explicitly discarding the result should more naturally handle the talk's pitfalls.

@JohelEGP JohelEGP force-pushed the requires-expression branch from 08afde6 to 1554ce3 Compare October 4, 2023 00:28
@JohelEGP JohelEGP force-pushed the requires-expression branch 2 times, most recently from 90da791 to f6d0ff5 Compare October 30, 2023 20:27
Comment on lines +7160 to +8188
//G simple-requirement
//G type-id ';' // Cpp1 type-requirement
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that this should simply be

Suggested change
//G simple-requirement
//G type-id ';' // Cpp1 type-requirement
//G type-id ';' // Cpp1 type-requirement
//G expression ';' // Cpp1 simple-requirement

Just like the rhs of is-as-expression,
potentially template arguments after resolving #727, and
potentially the lhs of is-as-expression after merging #782.

That's consistent, but would also make it easier to reintroduce the pitfalls.

// https://youtu.be/CXn02MPkn8Y?t=2337
negatable: <T> concept = requires(t: T)
{
  (-t is T); // Hopefully obviously wrong. Should be `{ -t } is T;`.
  -t is T;   // Wrong.
};

// https://youtu.be/CXn02MPkn8Y?t=2418
int_sized: <T> concept = requires(t: T)
{
  (sizeof(T) == 4); // Hopefully obviously wrong. Should be `requires (sizeof(T) == 4);`.
};
// Could also be `int_sized: <T> concept = sizeof(T) == 4;`.

// https://youtu.be/CXn02MPkn8Y?t=2455
nothrow_incrementable: <T> concept = requires(inout t: T)
{
  (noexcept(t++)); // Hopefully obviously wrong. Should be `requires noexcept(t++);` or `{ t++ } !throws;`.
};

// https://quuxplusone.github.io/blog/2021/06/09/another-concepts-chest-mimic/
has_a_but_not_b: <T> concept = requires(t: T)
{
  (a(t));
  !requires (b(t)); // In Cpp2, this works and does the correct thing.
  (!requires (b(t))); // Wrong.
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still be done for consistency.
And the pitfalls can still be prevent by going back to #588 (reply in thread).
Which means lowering -t is T; to { -t is T } -> not_boolean_testable.
We should also lower (!requires (b(t))) similarly,
or all simple-requirements as the linked comment says.

@JohelEGP JohelEGP force-pushed the requires-expression branch from b5dda79 to 698b44c Compare October 9, 2024 16:34
@hsutter
Copy link
Owner

hsutter commented Oct 31, 2024

Hi! Sorry it took me so long to get to this one... but this looks like it might still be current.

Is this up to date, and would you like me to review it?

@hsutter hsutter added the question - further information requested Further information is requested label Oct 31, 2024
@JohelEGP
Copy link
Contributor Author

Yes, and yes, please do.

@@ -1 +1,2 @@
pure2-concept-definition.cpp
pure2-concept-definition.cpp2(28): error C2607: static assertion failed
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only regression test glitch I noticed ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never noticed, since I don't use MSVC.
Somehow this is failing: static_assert(!test_nonthrowing_requirements<std::chrono::seconds>);.

@hsutter
Copy link
Owner

hsutter commented Nov 2, 2024

This would also need documentation -- would you add some coverage of this under /docs please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants