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

array-to-raw-elem cast: test that Retag covers entire array #85023

Merged
merged 1 commit into from
May 11, 2021

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented May 7, 2021

Make sure that we Retag before doing the ArrayToPointer cast.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 7, 2021
Retag(_4); // scope 1 at $DIR/retag.rs:59:13: 59:19
_3 = &raw mut (*_4); // scope 1 at $DIR/retag.rs:59:13: 59:19
Retag([raw] _3); // scope 1 at $DIR/retag.rs:59:13: 59:19
_2 = move _3 as *mut usize (Pointer(ArrayToPointer)); // scope 1 at $DIR/retag.rs:59:13: 59:33
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key part of the test. I am not sure how to make sure this won't be bless'ed away...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I wonder if we want something like LLVM's FileCheck for mir-opt tests... cc @rust-lang/wg-mir-opt do we have any standard practice here?

@Mark-Simulacrum
Copy link
Member

r=me in theory, though I agree this seems error-prone as-is.

@RalfJung
Copy link
Member Author

@oli-obk I recall you working on this infrastructure so that mir-opt tests can be blessed; is there any way to make sure that the test actually keeps testing what it is supposed to test though? The retag mir-opt test as it exists currently in master is a lot more fragile that I'd like it to be (and back when I added the test, it was less fragile, back then it did ensure that there'd be Retags at the right places -- something got removed in the mean time).

@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2021

There is no existing infrastructure, but it would be possible to add. I think the hard part is knowing what we actually want.

We could do something simple like requiring that a certain string exists in the mir dump, that could easily be added to compiletest by adding more magic comments like the ones for actually doing a dump.

@RalfJung
Copy link
Member Author

I opened #85180 for that.

Looks like we can't really do better for this PR currently, so
@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented May 11, 2021

📌 Commit 2d2ed21 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2021
@bors
Copy link
Contributor

bors commented May 11, 2021

⌛ Testing commit 2d2ed21 with merge 2bafe96...

@bors
Copy link
Contributor

bors commented May 11, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 2bafe96 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 11, 2021
@bors bors merged commit 2bafe96 into rust-lang:master May 11, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 11, 2021
@RalfJung RalfJung deleted the array-to-raw-elem branch May 11, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants