allow numbers with provenance within CTFE execution #99965
Conversation
This effectively reverts rust-lang#97684 for CTFE
rustbot
added
the
T-compiler
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
|
rust-highfive
added
the
S-waiting-on-review
|
Or should someone from @rust-lang/wg-const-eval review this? @oli-obk is on vacation this week and next week it is too late to merge this... |
|
I think it would be good to get some more compiler-y eyes on this, yes - not sure who a good candidate would be. |
Mark-Simulacrum
added
the
beta-nominated
|
beta-nominating, since this is ultimately a beta backport of sorts. I'd also personally like to see a similar revert on master, so that we have ~12 weeks to discuss and not ~6 (since master branches into beta in ~6 days). |
That would be a much bigger endeavor, since |
|
Ah, OK. In that case we should make sure that the discussion concludes before 1.64 releases, presuming this gets merged :) |
| fn enforce_number_no_provenance(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { | ||
| true | ||
| false |
There was a problem hiding this comment.
Was this only read during "CTFE validation", before? Could self.ctfe_mode.is_some() calls be moved into here instead?
There was a problem hiding this comment.
Was this only read during "CTFE validation", before?
Yes. #97684 made it also relevant for read_immediate.
Could self.ctfe_mode.is_some() calls be moved into here instead?
No; the self in validation is the validation visitor, and that has the ctfe_mode -- but that doesn't exist in read_immediate.
There was a problem hiding this comment.
I guess alternatively in this PR we could make enforce_number_no_provenance again only relevant for validation, and not for read_immediate. That would break Miri but we don't care about Miri on the beta branch.
|
So do we want to land this? We don't have that much time left... |
rustbot
added
the
beta-accepted
|
So can we ask bors to land this now? @Mark-Simulacrum I don't know the process for beta PRs. |
|
@bors r+ (hopefully enough) |
bors
added
S-waiting-on-bors
|
@bors p=10 |
|
☀️ Test successful - checks-actions |
Mark-Simulacrum
removed
the
beta-nominated
…=pnkfelix beta-backport of provenance-related CTFE changes This is all part of dealing with rust-lang#99923. The first commit backports the effects of rust-lang#101101. `@pnkfelix` asked for this and it turned out to be easy, so I think this is uncontroversial. The second commit effectively repeats rust-lang#99965, which un-does the effects of rust-lang#97684 and therefore means rust-lang#99923 does not apply to the beta branch. I honestly don't think we should do this; the sentiment in rust-lang#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.
This effectively reverts #97684 for CTFE.
Undoes the diagnostic changes that are tracked in #99923, only for beta.
(On master this patch wouldn't apply any more,
enforce_number_no_provenanceis gone with #99644 since the interpreter engine is not supposed to ever have provenance on integers.)The test changes are an exact un-do of #97684. However there is still some risk here since this exact code is not what has been battle-tested.
r? @Mark-Simulacrum