Skip to content

Consider making num.clamp generic? #30403

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

Closed
leafpetersen opened this issue Aug 10, 2017 · 7 comments
Closed

Consider making num.clamp generic? #30403

leafpetersen opened this issue Aug 10, 2017 · 7 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 library-core

Comments

@leafpetersen
Copy link
Member

Currently num.clamp is defined as (num, num) -> num. Making this generic would allow better static typing.

cc @floitschG @lrhn

@leafpetersen leafpetersen added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Aug 10, 2017
@leafpetersen
Copy link
Member Author

See #30402 for some context.

@lrhn
Copy link
Member

lrhn commented Aug 11, 2017

It won't be easy to do that and preserve the current behavior, or maybe doing it at all.

What signature would you suggest for the method?

T clamp<T extends num>(T low, T high);  

This won't work.
The call x.clamp(y, z) can return any of x, y or z. That means that num x = 42; x.clamp(0.0, 100.0); will likely infer T as double and complain that the return value is int.

The problem is that the actual constraint is that T must be a type that both arguments and the value itself can be assigned to. We cannot express that constraint.

Even if we specialize the method for int and double, so we basically know the type of this, we still need:

T clamp<T extends ??>(T low, T high);

where ?? somehow restricts T to int and num, but avoids double

Maybe just add

int int.clampInt(int low, int high);
double double.clampDouble(double low, double high);

?

@leafpetersen
Copy link
Member Author

leafpetersen commented Aug 15, 2017

Hmm, yeah, unfortunate. Overloading might help here, if you could use the type of the receiver.

Another option would be:

//num
num clamp(covariant num low, covariant num high);

//int
@override
int clamp(int low, int high)

@lrhn
Copy link
Member

lrhn commented Aug 16, 2017

Using covariant wouldn't work very well either.
In practice, it means that you have int int.clamp(int,int) and double double.clamp(double, double), but there is no way you can call the clamp of num x; without testing the type of it first, and then you could just remove the num.clamp declaration completely and not be any worse off.

I think the best solution is to remove clamp from num and put it on int and double, or to keep it on num and put clampInt and clampDouble on int and double.

Or remove it from num and make it a static function T clamp<T extends num>(T value, T min, T max). Then it can be used with both int, double and num.
(I don't see a good way to get "self" into the type system, short of adding self-types like This clamp<T extends This>(T min, T max)).

@floitschG
Copy link
Contributor

We are dropping clamp from num and specialize the functions on int and double.

@floitschG floitschG added core-2 and removed core-m labels Aug 21, 2017
@lukepighetti
Copy link

Friendly ping for forward movement on this. Spent a few hours debugging an issue with num.clamp() on a double when I was expecting a double out the other side.

@lrhn
Copy link
Member

lrhn commented Oct 3, 2020

We will allow better static typing through special casing in the type system.

@lrhn lrhn closed this as completed Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 library-core
Projects
None yet
Development

No branches or pull requests

5 participants