-
Notifications
You must be signed in to change notification settings - Fork 252
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
base: main
Are you sure you want to change the base?
Conversation
I find it weird that these use different kind of brackets. Since, compound expressions are just simple expressions with |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
To simplify that, in addition to
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
To compare: requires requires(const T& c) { c - c; } // Cpp1
requires !requires { typename std::iter_reference_t<T>; } // Cpp1
|
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. |
A requires clause can only happen before the |
Yes, but it still can be confusing to the reader. |
I will doubt that until I find out code that makes me think so. |
Fair enough 👌 |
I'm changing this #526's and #596's use of With that change, you won't need to add |
c745753
to
840ac3e
Compare
840ac3e
to
08afde6
Compare
08afde6
to
1554ce3
Compare
90da791
to
f6d0ff5
Compare
//G simple-requirement | ||
//G type-id ';' // Cpp1 type-requirement |
There was a problem hiding this comment.
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
//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.
};
There was a problem hiding this comment.
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.
f6d0ff5
to
b9cd2a2
Compare
b9cd2a2
to
0eb8cb8
Compare
0eb8cb8
to
b9f51de
Compare
b9f51de
to
b5dda79
Compare
b5dda79
to
698b44c
Compare
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? |
Yes, and yes, please do. |
@@ -1 +1,2 @@ | |||
pure2-concept-definition.cpp | |||
pure2-concept-definition.cpp2(28): error C2607: static assertion failed |
There was a problem hiding this comment.
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 ... ?
There was a problem hiding this comment.
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>);
.
This would also need documentation -- would you add some coverage of this under |
This PR adds support for requires-expressions (first based on #588 (comment)).
This is the syntax, based on Cpp1's:
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:
if constexpr (!requires { (t.begin()); })
toif constexpr (!requires t.begin();)
.requires
expressions #596 (comment).