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

proposal: x/exp/constraints: move into math, sort #52427

Closed
earthboundkid opened this issue Apr 19, 2022 · 14 comments
Closed

proposal: x/exp/constraints: move into math, sort #52427

earthboundkid opened this issue Apr 19, 2022 · 14 comments

Comments

@earthboundkid
Copy link
Contributor

This is follow up to #50792.

Most of the remaining constraints after #48424 are purely numeric: Complex, Float, Integer, Signed, Unsigned. They can all just be added to package math. Ordered is the exception because it also involves strings. It can be added to package sort. There's no longer any need for a top level constraints package.

@gopherbot gopherbot added this to the Proposal milestone Apr 19, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Apr 19, 2022
@ianlancetaylor
Copy link
Contributor

I'm not sure that math.Integer or sort.Ordered convey quite the right feel. They don't sound like constraints.

@earthboundkid
Copy link
Contributor Author

I think sort.Orderable reads a little better than sort.Ordered.

I think the math ones read fine though: func Sum[N math.Integer|math.Float](ns ...N) N.

@zephyrtronium
Copy link
Contributor

I'm not sure that math.Integer or sort.Ordered convey quite the right feel. They don't sound like constraints.

Conversely, as I mentioned in #45458 (comment), constraints might not always remain only constraints. Committing to names like constraints.Float instead of math.Float could make the future more awkward.

@changkun
Copy link
Member

I think this is a good one to avoid the lengthy constraints to appear in the function signature.

I have been experimenting with roughly the same idea that uses self-defined math.Float to replace constraints.Float (also intended to avoid import x/exp/constraints just for a simple interface definition). My overall feeling on the usage of math.Float is positive. See how the usage is distributed in a math package.

The key questions: do we really need a centralized place to define "all" constraints? or how many constraints are expected to appear but not yet in x/exp/constraints?

@changkun
Copy link
Member

One addition but less problematic: the dependencies of math and sort won't be able to use these constraints (i.e. math/bits and runtime), but they can also define these constraints internally to avoid circular import easily.

@mpx
Copy link
Contributor

mpx commented Apr 21, 2022

Perhaps it's a coincidence that the current constaints can be considered for the math package? Other future constaints might not have a convient pre-existing package to slot them into. There might be a need for the remainder to fit into a separate "constraints" package again?

This is hypothetical, but it's hard to predict where type parameters and constraints will evolve over time - developers have only started exploring this space.

Is this mostly a workaround for constraints being too long? (I don't think that is a good reason). Or do people think merging into existing packages is more appropriate for a reason outside package name length?

@earthboundkid
Copy link
Contributor Author

I think experience is showing that you don't really need that many constraints, except for Ordered or (and this isn't in the exp/constraints anyway) byteseq.

"Constraints" is like "utils" but for types. It's better to have something specific than a bunch of general stuff crammed in one place.

@rsc rsc changed the title proposal: merge golang.org/x/exp/constraints into math, sort proposal: x/exp/constraints: move into math, sort Aug 10, 2022
@beoran
Copy link

beoran commented Dec 31, 2022

The main problem with a constraints package is that is is very likely to become a dependency of all generic packages, even though it doesn't have any functionality, only types. But "a little copying is better than a little dependency", so it is probably better for each generic package to define its own constraints.

@earthboundkid
Copy link
Contributor Author

Ordered is the only constraint that’s non-trivial to define for yourself. It would be convenient to have it in a canonical place, like sort, if only to copy it elsewhere.

@ianlancetaylor
Copy link
Contributor

@beoran It would be interesting to see how many packages that use generics do in fact import constraints. I would be surprised if most do. I checked some packages in Google internal code that define generics, and none of them import constraints.

@kylelemons
Copy link
Contributor

kylelemons commented May 19, 2023

My team just went down the rabbit hole of history today trying to track down the new home of x/exp/slices.SortFunc (after discovering a major performance win from moving from sort.Slice to sort.Sort and wondering if a generic version of sort.Slice would've been faster), and if I'm reading the closed proposals correctly it seems like this issue is the current "blocker" for that functionality going into the standard library -- we need constraints.Ordered or sort.Ordered or something equivalent before we can get slices.Sort. Is that correct? If so, can we get an update on the status of this proposal?

@zephyrtronium
Copy link
Contributor

zephyrtronium commented May 19, 2023

@kylelemons #60091

(Interesting that there wasn't already a link between these issues.)

@jimmyfrasche
Copy link
Member

As long as we're cross-indexing:

#59488 is for cmp.Ordered instead of sort.Ordered and that's likely to succeed.

Much less officially, in #60274 (comment) there's an alternative to putting the numeric constraints in math.

@earthboundkid
Copy link
Contributor Author

Closing this issue because #59488 was accepted and implemented. Math constraints are part of #60274 or they can have a separate proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

9 participants