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

Fix MIR typeck soundness holes #69145

Merged
merged 2 commits into from
Feb 20, 2020
Merged

Fix MIR typeck soundness holes #69145

merged 2 commits into from
Feb 20, 2020

Conversation

matthewjasper
Copy link
Contributor

  • Check types of static items
  • Always check lifetime bounds of Copy impls

r? @nikomatsakis
closes #69114

@matthewjasper matthewjasper added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Feb 13, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 13, 2020
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 14, 2020
@pnkfelix
Copy link
Member

stealing review from niko

@pnkfelix pnkfelix self-assigned this Feb 20, 2020
nikomatsakis
nikomatsakis approved these changes Feb 20, 2020
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

r=me

src/librustc_mir/borrow_check/type_check/mod.rs
@@ -503,6 +490,31 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
place_ty = self.sanitize_projection(place_ty, elem, place, location)
}

if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context {
Copy link
Contributor

Choose a reason for hiding this comment

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

The main change here is that we now do this for all places, right? I wonder why we didn't before? That just seems like a silly oversight to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 20, 2020

📌 Commit ddc2545 has been approved by nikomatsakis

@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 Feb 20, 2020
@Centril
Copy link
Contributor

Centril commented Feb 20, 2020

@bors p=10 rollup=never

@pnkfelix
Copy link
Member

discussed in T-compiler meeting.

accepted for beta and stable backport.

however, please do wait until this lands in nighty before initiating such a backport...

@pnkfelix pnkfelix added beta-accepted Accepted for backporting to the compiler in the beta channel. stable-accepted Accepted for backporting to the compiler in the stable channel. labels Feb 20, 2020
eddyb
eddyb reviewed Feb 20, 2020
View reviewed changes
src/librustc_mir/borrow_check/type_check/mod.rs
ConstraintCategory::Boring,
) {
span_mirbug!(self, constant, "bad static type {:?} ({:?})", constant, terr);
}
Copy link
Member

Choose a reason for hiding this comment

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

This is another thing where I don't understand why the new static representation doesn't result in correctness by default.

Copy link
Member

Choose a reason for hiding this comment

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

Could MIR building inject a UserTypeAnnotation for static references/pointers? (note that I don't want to block this PR, any fix here should be equivalent, so we can clean it up later)

@bors
Copy link
Contributor

bors commented Feb 20, 2020

⌛ Testing commit ddc2545 with merge bfb9604...

@bors
Copy link
Contributor

bors commented Feb 20, 2020

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing bfb9604 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 20, 2020
@bors bors merged commit bfb9604 into rust-lang:master Feb 20, 2020
@matthewjasper matthewjasper deleted the mir-typeck-static-ty branch February 20, 2020 19:27
This was referenced Feb 21, 2020
Merged
Merged
@Mark-Simulacrum Mark-Simulacrum removed beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Feb 21, 2020
bors added a commit that referenced this pull request Feb 22, 2020
@bors
Auto merge of #69359 - Mark-Simulacrum:stable-next, r=<try>
9ab2cd6 
[stable] 1.41.1 release

This backports the following PRs:
 * Revert "Remove `checked_add` in `Layout::repeat`" #69241
 * Do not ICE when encountering `yield` inside `async` block #69175
 * Correct ICE caused by macros generating invalid spans. #68611
 * Changelog: Demonstrate final build-override syntax #68603
 * Resolve long compile times when evaluating always valid constants #67667
 * Fix MIR typeck soundness holes #69145

This also includes a commit which rustfmt's files which the latter commits touched (and perhaps a bit more) to make rebasing the PRs go more smoothly (thankfully, this should be the last time we need to do so).

I have removed stable-nominated tags from PRs successfully backported.
bors added a commit that referenced this pull request Feb 24, 2020
@bors
Auto merge of #69359 - Mark-Simulacrum:stable-next, r=Mark-Simulacrum
ded0270 
[stable] 1.41.1 release

This backports the following PRs:
 * Revert "Remove `checked_add` in `Layout::repeat`" #69241
 * Do not ICE when encountering `yield` inside `async` block #69175
 * Correct ICE caused by macros generating invalid spans. #68611
 * Changelog: Demonstrate final build-override syntax #68603
 * Resolve long compile times when evaluating always valid constants #67667
 * Fix MIR typeck soundness holes #69145

This also includes a commit which rustfmt's files which the latter commits touched (and perhaps a bit more) to make rebasing the PRs go more smoothly (thankfully, this should be the last time we need to do so).

I have removed stable-nominated tags from PRs successfully backported.
bors added a commit that referenced this pull request Feb 24, 2020
@bors
Auto merge of #69360 - Mark-Simulacrum:beta-next, r=Mark-Simulacrum
13e0fbc 
[beta] beta backports

This backports the following PRs:

* Revert "Remove `checked_add` in `Layout::repeat`" #69241
* Do not ICE when encountering `yield` inside `async` block #69175
* Fix MIR typeck soundness holes #69145
* Fix extra subslice lowering #69128
* Correct ICE caused by macros generating invalid spans. #68611
* Make conflicting_repr_hints a deny-by-default c-future-compat lint #68586
@niklasad1 niklasad1 mentioned this pull request Feb 27, 2020
Closed
@jonas-schievink jonas-schievink mentioned this pull request Mar 30, 2020
Closed
@QuineDot QuineDot mentioned this pull request Jun 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb left review comments

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

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

Successfully merging this pull request may close these issues.

Static lifetimes checking regression in rustc 1.41.0
9 participants
@matthewjasper @pnkfelix @nikomatsakis @bors @Centril @eddyb @jonas-schievink @Mark-Simulacrum @rust-highfive