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

Implement Ord/PartialOrd? #114

Closed
alexcrichton opened this issue May 30, 2015 · 6 comments
Closed

Implement Ord/PartialOrd? #114

alexcrichton opened this issue May 30, 2015 · 6 comments

Comments

@alexcrichton
Copy link
Contributor

I notice that currently these traits aren't implemented for Url, and I was curious if this was done on purpose? If not, would it be possible to add them?

@SimonSapin
Copy link
Member

I don’t think it makes sense to consider an URL intrinsically "greater" or "lesser" than another. But what’s the use case?

@alexcrichton
Copy link
Contributor Author

The need for this is derived from Cargo, which primarily uses it for deterministic output in a few locations. There are spots where we sort an array of PackageId (which transitively contains a Url at some point) to ensure that the output is consistent among stages. Put another way, the order itself isn't super important so long as it's consistent.

I personally view core traits like this as being useful to define on core types like Url, even when they don't always make 100% sense (e.g. PartialEq for RefCell)

@SimonSapin
Copy link
Member

Ok, that sounds like a real use case and I don’t have better way to solve it. I still don’t think any ordering is meaningful for URLs, but adding a few #[derive(PartialOrd)] is not harmful either.

I don’t get your second point, though. What qualifies as a core trait? Should Url implement Add, for example? What qualifies as a core type? What makes Url more "core" than, say, cssparser::RGBA? As to RefCell, since it’s a container it doesn’t need to have a meaningful equality by itself, it just defers to its contents.

@alexcrichton
Copy link
Contributor Author

Ah I just mean "core" in the sense that it's often a building block for many other types. For example there are probably going to be a lot of structs which store a Url, some of which may want to have some sort of ordering eventually.

@seanmonstar
Copy link
Contributor

As for whether Ordering makes sense for Url: it seems natural that it would
be the same as Ord for strings: likely alphabetically.

On Mon, Jun 1, 2015, 11:08 AM Alex Crichton notifications@github.com
wrote:

Ah I just mean "core" in the sense that it's often a building block for
many other types. For example there are probably going to be a lot of
structs which store a Url, some of which may want to have some sort of
ordering eventually.


Reply to this email directly or view it on GitHub
#114 (comment).

@SimonSapin
Copy link
Member

Ordering URLs by the string ordering of their serialization seems reasonable, but it’s not the same ordering as what #[derive(PartialOrd)] would get us with the current internal representation. Still, we should probably stick with derive, to avoid performance issues like #113.

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

No branches or pull requests

3 participants