Skip to content

PartialOrd for opaque? #1013

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

Open
pepyakin opened this issue Sep 22, 2017 · 2 comments
Open

PartialOrd for opaque? #1013

pepyakin opened this issue Sep 22, 2017 · 2 comments

Comments

@pepyakin
Copy link
Contributor

In #882 (PR #1002) we used PartialEq analysis to analyse whether we can derive PartialOrd. Generally this makes sense because if PartialEq can be derived then PartialOrd can be derived too.

But there is a problem. I can imagine what means for opaque types to be PartialEq: we just have to compare byte-by-byte both blobs. But I have a problem imaging what we should do in case of PartialOrd: compare each byte in both blobs?

Maybe we should split result of the analysis and explicitly state whether we can derive PartialOrd or not?

@pepyakin
Copy link
Contributor Author

Writing this down I realized that it makes no sense to derive PartialEq for blobs either!
I'm not a compiler wizard, but isn't it is at least incorrect (if not UB) to compare byte-by-byte two structs because of stuff like padding bytes which, if I understand correctly, have undefined values.

@fitzgen
Copy link
Member

fitzgen commented Sep 22, 2017

Good point about padding. I guess we should only be "manually" implementing these things so that we don't look into padding.

Or I guess we could define a special type for padding that implements various traits with some kind of default/no-op that doesn't actually read the padding bits, and then we could still derive and the padding would effectively be skipped over.

This doesn't apply for opaque blobs of course.


@emilio if we stopped deriving traits for opaque things (except maybe Debug?), would that have bad implications for Stylo?

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

No branches or pull requests

2 participants