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

[range.split.view] Make require-constant a function #4423

Closed
JohelEGP opened this issue Dec 22, 2020 · 4 comments
Closed

[range.split.view] Make require-constant a function #4423

JohelEGP opened this issue Dec 22, 2020 · 4 comments

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Dec 22, 2020 •
edited

This implementation results in a single instantiation per type, rather than one specialization per value (do compilers cache these when the template's not defined?)

-  template<auto> struct require-constant;       // exposition only
+  consteval void require-constant(auto) {}      // exposition only

   template<class R>
   concept tiny-range =                          // exposition only
     sized_range<R> &&
-    requires { typename require-constant<remove_reference_t<R>::size()>; } &&
+    requires { require-constant(remove_reference_t<R>::size()); } &&
     (remove_reference_t<R>::size() <= 1);

or (lambdas are not SCARY, are they?)

-  template<auto> struct require-constant;       // exposition only
-
   template<class R>
   concept tiny-range =                          // exposition only
     sized_range<R> &&
-    requires { typename require-constant<remove_reference_t<R>::size()>; } &&
+    requires { [](auto) consteval {}(remove_reference_t<R>::size()); } &&
     (remove_reference_t<R>::size() <= 1);

See https://godbolt.org/z/svc9Gx.
@JohelEGP JohelEGP changed the title [range.split.view] Replace require-constant with require-consteval [range.split.view] Make require-constant a function Dec 22, 2020
@timsong-cpp
Copy link
Contributor

I don't understand the point in trying to optimize the compile time of the definition of an exposition-only concept like this. The exact definition is not observable; the implementers are free to optimize it already.

In any event, I don't think this is correct. Expressions in a requires-expression are not potentially-evaluated, so the consteval doesn't do anything.

@JohelEGP
Copy link
Contributor Author

I don't understand the point in trying to optimize the compile time of the definition of an exposition-only concept like this. The exact definition is not observable; the implementers are free to optimize it already.

Definitely. I thought I'd make this more visible. And also get experts to tell me whether my variation of require-constant in my code is actually well-formed. :)

In any event, I don't think this is correct. Expressions in a requires-expression are not potentially-evaluated, so the consteval doesn't do anything.

Well, maybe I found a bug in these compilers that pass the tests.

@timsong-cpp
Copy link
Contributor

That test isn't testing whether T::size() is a constant. You are testing whether it is well-formed.

@JohelEGP
Copy link
Contributor Author

That test isn't testing whether T::size() is a constant. You are testing whether it is well-formed.

This sounds very familiar. I must have repeated this mistake. Thank you, that makes sense.

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
@timsong-cpp @JohelEGP