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

Do not attempt to ascribe projections out of a ty var #55637

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Nov 3, 2018

If we encounter _ ascribed to structural pattern like (a, b), just skip relate_types.

Fix #55552

@rust-highfive
Copy link
Collaborator

r? @varkor

(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 Nov 3, 2018
@pnkfelix pnkfelix changed the title Dont attempt to ascribe projections out of a ty var Do not attempt to ascribe projections out of a ty var Nov 3, 2018
@pnkfelix pnkfelix added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 3, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 3, 2018

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned varkor Nov 3, 2018
self,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
elem: &ProjectionElem<'tcx, V, T>,
mut handle_field: impl FnMut(&Self, &Field, &T) -> Result<Ty<'tcx>, E>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: rustfmt would put the ) on the next line.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2018

📌 Commit 1bbaa55 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 Nov 6, 2018
@nikomatsakis
Copy link
Contributor

@pnkfelix and I were discussing this PR and we observed that it could cause problems if you have repeated type variables, e.g., type Foo<T> = (T, T) and an annotation like Foo<_>

@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 8, 2018

discussed at T-compiler meeting. beta-accepting (with a wee bit of trepidation from @nagisa )

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 8, 2018
@pietroalbini
Copy link
Member

@bors p=1 (beta accepted)

@bors
Copy link
Contributor

bors commented Nov 9, 2018

⌛ Testing commit 1bbaa55 with merge 041e18f00bcde960a131884b391f59a077a6b5a3...

@bors
Copy link
Contributor

bors commented Nov 9, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 9, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 9, 2018

---- support::resolver::meta_test_multiple_versions_strategy stdout ----
thread 'support::resolver::meta_test_multiple_versions_strategy' panicked at 'In 640 tries we did not see a wide enough distribution of multiple versions of the same library! dis: [387, 79, 51, 20, 16, 15, 2, 0, 3, 12]', tools\cargo\tests\testsuite\support\resolver.rs:484:5
failures:
    support::resolver::meta_test_multiple_versions_strategy
test result: FAILED. 1440 passed; 1 failed; 4 ignored; 0 measured; 0 filtered out
error: test failed, to rerun pass '--test testsuite'

... that message ... sounds like a spurious failure to me ...?

@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 9, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Nov 9, 2018

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Nov 9, 2018

📌 Commit 1bbaa55 has been approved by pnkfelix

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 9, 2018
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 9, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 9, 2018

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 9, 2018

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Nov 9, 2018

📌 Commit 1bbaa55 has been approved by nikomatsakis

@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 9, 2018

@bors retry

@pnkfelix
Copy link
Member Author

pnkfelix commented Nov 9, 2018

@bors p=1 (beta accepted)

Copy link
Member

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

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

Noticed some spell errors.

let mut projected_ty = PlaceTy::from_ty(ty);
let ty = self.normalize(ty, locations);

// We need to follow any provided projetions into the type.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// We need to follow any provided projetions into the type.
// We need to follow any provided projections into the type.

@@ -151,17 +151,35 @@ impl AscribeUserTypeCx<'me, 'gcx, 'tcx> {
debug!("relate_type_and_user_type: ty of def-id is {:?}", ty);
let ty = self.normalize(ty);

let mut projected_ty = PlaceTy::from_ty(ty);
// We need to follow any provided projetions into the type.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// We need to follow any provided projetions into the type.
// We need to follow any provided projections into the type.


// rust-lang/rust#55552: The strategy pnkfelix landed in PR #55274
// (for ensuring that NLL respects user-provided lifetime annotations)
// did not handle the case where the ascribed type has some expliit
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// did not handle the case where the ascribed type has some expliit
// did not handle the case where the ascribed type has some explicit

@bors
Copy link
Contributor

bors commented Nov 10, 2018

⌛ Testing commit 1bbaa55 with merge 4cd3294...

bors added a commit that referenced this pull request Nov 10, 2018
…rojections-out-of-a-ty-var, r=nikomatsakis

Do not attempt to ascribe projections out of a ty var

If we encounter `_` ascribed to structural pattern like `(a, b)`, just skip relate_types.

Fix #55552
@bors
Copy link
Contributor

bors commented Nov 10, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 4cd3294 to master...

@bors bors merged commit 1bbaa55 into rust-lang:master Nov 10, 2018
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 10, 2018
bors added a commit that referenced this pull request Nov 10, 2018
[beta] Rollup backports

Merged and approved:

* #55831: [beta]: Update Cargo's submodule
* #55717: Bubble up an overflow error so that rustdoc can ignore it
* #55637: Do not attempt to ascribe projections out of a ty var
* #55378: rustbuild: use configured linker to build bootstrap

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.

7 participants