[WIP] Preliminary work splitting const qualification into separate passes #52518
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
oli-obk
reviewed
Jul 19, 2018
| @@ -101,7 +101,9 @@ struct Qualifier<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { | |||
| rpo: ReversePostorder<'a, 'tcx>, | |||
| tcx: TyCtxt<'a, 'gcx, 'tcx>, | |||
| param_env: ty::ParamEnv<'tcx>, | |||
| local_qualif: IndexVec<Local, Option<Qualif>>, | |||
There was a problem hiding this comment.
The commit message isn't up to date anymore. The temp + return merge is already in master
src/librustc/mir/interpret/error.rs
Outdated
| @@ -175,6 +176,7 @@ pub enum EvalErrorKind<'tcx, O> { | |||
| NoMirFor(String), | |||
| UnterminatedCString(Pointer), | |||
| DanglingPointerDeref, | |||
| ExternStaticRead(DefId), | |||
There was a problem hiding this comment.
This is introduced here and removed two commits later. Merge the commits into each other?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
r? @eddyb |
This comment has been minimized.
This comment has been minimized.
oli-obk
reviewed
Jul 21, 2018
| let store = |this: &mut Self, index| { | ||
| this.local_mut_interior.set_member(&index, this.qualif.mut_interior); | ||
| this.local_needs_drop.set_member(&index, this.qualif.needs_drop); | ||
| this.local_not_const.set_member(&index, this.qualif.not_const); | ||
| }; | ||
|
|
||
| // Only handle promotable temps in non-const functions. | ||
| if self.mode == Mode::Fn { | ||
| if let Place::Local(index) = *dest { | ||
| if self.mir.local_kind(index) == LocalKind::Temp |
There was a problem hiding this comment.
I believe the failure here is due to this not also entering this arm in case of LocalKind::ReturnPointer. Although I don't see how that worked before!?
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
… vectors for qualif flags.
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
alexreg
commented
Jul 22, 2018
| _: Location) { | ||
| let kind = self.mir.local_kind(local); | ||
| match kind { | ||
| LocalKind::ReturnPointer => {} |
There was a problem hiding this comment.
And it should still be self.not_const() in the ConstQualifier pass, I presume?
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
…drop, const checker).
TimNN
removed
the
S-waiting-on-review
|
Ping from triage, @alexreg. It looks like some changes have been requested to your PR. |
|
@TimNN Yeah, I was away on holiday, but just getting to them now. Hopefully will have them resolved in the next day or two. :-) |
|
Triage: @alexreg Hope you had a good holiday. Can you give a status update on this PR? |
|
It looks like @eddyb is needed: #49146 (comment) |
XAMPPRocky
added
S-waiting-on-review
|
@mark-i-m Thank you, I've updated the status of the PR to reflect this. |
|
@Aaronepower Yep. I've been pestering @eddyb about this for a while. I think he'll finally get around to it soon. :-) |
|
Ping from triage @eddyb! This PR needs your review. |
|
Ping from triage @eddyb / @rust-lang/compiler: This PR requires your review. |
1 similar comment
|
Ping from triage @eddyb / @rust-lang/compiler: This PR requires your review. |
|
Conflict ahead: #54424 |
TimNN
added
A-allocators
|
Ping from triage @eddyb / @rust-lang/compiler: What are your plans for this PR? |
|
Side note: please do not leave merge commits in the PR. |
|
@estebank That's a rebase gone wrong I think. :-P |
|
Ping from triage @eddyb: This PR requires your review! |
|
Ping from triage @eddyb / @rust-lang/compiler! This PR requires your review. |
Centril
added
S-waiting-on-team
|
Ping from triage @eddyb / @rust-lang/compiler -- we are eagerly awaiting your review. |
apiraino
removed
the
S-waiting-on-team
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @eddyb
CC @oli-obk