Skip to content

fix comparing wide raw pointers #2336

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

Merged
merged 1 commit into from
Jul 6, 2022
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 6, 2022

Fixes rust-lang/rust#96169

However I am not sure if these are the correct semantics. Lexicographic comparison of the two wide ptr components?!? I'll wait for confirmation in that issue.

@RalfJung RalfJung force-pushed the wide-ptr-compare branch 2 times, most recently from 0c48ba8 to 628f847 Compare July 6, 2022 01:19
@RalfJung RalfJung force-pushed the wide-ptr-compare branch from 628f847 to 6c8ad4a Compare July 6, 2022 01:21
@RalfJung
Copy link
Member Author

RalfJung commented Jul 6, 2022

Confirmed by @bjorn3

@JakobDegen we probably want this in the MIR semantics docs.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 6, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 6, 2022

📌 Commit 6c8ad4a has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Jul 6, 2022

⌛ Testing commit 6c8ad4a with merge 36d8f5c...

@bors
Copy link
Contributor

bors commented Jul 6, 2022

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 36d8f5c to master...

@bors bors merged commit 36d8f5c into rust-lang:master Jul 6, 2022
@RalfJung RalfJung deleted the wide-ptr-compare branch July 6, 2022 13:36
@JakobDegen
Copy link
Contributor

@RalfJung this seems more like something that should go in the reference, no? It's a language thing after all, not MIR specific. We can put it in the docs too of course, but I don't generally think we need to duplicate language semantics there

@RalfJung
Copy link
Member Author

RalfJung commented Jul 7, 2022

It should be in the reference as well, yes.

Some MIR binops are surprisingly subtle though so I think it should be there, too.

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

Successfully merging this pull request may close these issues.

ICE: "Got a scalar pair where a scalar was expected" -Zmir-opt-level=4 --emit=mir
3 participants