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.

Sign up for GitHub

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

Jump to bottom

beta-backport of provenance-related CTFE changes #101320

Merged
merged 1 commit into from
Sep 9, 2022
Merged

beta-backport of provenance-related CTFE changes #101320

merged 1 commit into from
Sep 9, 2022

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 2, 2022

This is all part of dealing with #99923.

The first commit backports the effects of #101101. @pnkfelix asked for this and it turned out to be easy, so I think this is uncontroversial.

The second commit effectively repeats #99965, which un-does the effects of #97684 and therefore means #99923 does not apply to the beta branch. I honestly don't think we should do this; the sentiment in #99923 was that we should go ahead with the change but improve diagnostics. But @pnkfelix seemed to request such a change so I figured I would offer the option.

I'll be on vacation soon, so if you all decide to take the first commit only, then someone please just force-push to this branch and remove the 2nd commit.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 2, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 2, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive
Copy link
Collaborator

r? @cjgillot

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

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Sep 2, 2022 •
edited
Loading

I suppose I don't know what I want, and I'm trying to get enough information to decide what I want.

I don't want 1.64 to silently break the world; it seems obvious that we do have that, at least with respect to #99923. 👍

I also don't want 1.64 to loudly break the the world. Our crater runs indicate that we probably have that too. 👍

I don't want to re-litigate RFC rust-lang/rfcs#3016, (as some have suggested would be a an implication of reverting #97684 ), in a sudden/rash manner.

  • (I personally don't consider suggesting attempting a future-incompat cycle to be re-litigating that RFC; that RFC did not say we would require future-incompat cycles between changes to the UB detection, but it also didn't say we wouldn't. It left the development choices there unspecified, ha ha, and several project members such as myself didn't understand the implications thereof, given the constraints of the current CTfE code architecture until now.)

I am tempted to suggest that we go ahead and do exactly what @RalfJung would prefer here: land just the first commit in beta.

That way, we have the second commit in our back pocket if it turns out that 1.64.0 does cause significant breakage, then we do a quick 1.64.1 point release that adds the second commit. (But I'm assuming we are very unlikely to go down that path, so I'm not going to further muse on the implications of that choice on versions 1.65 and above.)

@RalfJung
Copy link
Member Author

RalfJung commented Sep 2, 2022

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned cjgillot Sep 2, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Sep 2, 2022 •
edited
Loading

I am tempted to suggest that we go ahead and do exactly what @RalfJung would prefer here: land just the first commit in beta.

All right, I removed the 2nd commit.

@apiraino
Copy link
Contributor

apiraino commented Sep 8, 2022

Beta backport discussed and accepted in compiler team meeting on Zulip

@rustbot label beta-nominated beta-accepted

@rustbot rustbot added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Sep 8, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Sep 8, 2022

@bors r

@bors
Copy link
Contributor

bors commented Sep 8, 2022

📌 Commit c7f8e68 has been approved by pnkfelix

It is now in the queue for this repository.

@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 Sep 8, 2022
@bors
Copy link
Contributor

bors commented Sep 9, 2022

⌛ Testing commit c7f8e68 with merge 25912c0...

@bors
Copy link
Contributor

bors commented Sep 9, 2022

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 25912c0 to beta...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 9, 2022
@bors bors merged commit 25912c0 into rust-lang:beta Sep 9, 2022
@rustbot rustbot added this to the 1.64.0 milestone Sep 9, 2022
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 19, 2022
@RalfJung RalfJung deleted the beta-ctfe-ptr-provenance branch March 9, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pnkfelix pnkfelix

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
8 participants
@RalfJung @rustbot @rust-highfive @pnkfelix @apiraino @bors @cjgillot @pietroalbini