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

[NLL] Type check operations with pointer types #58673

Merged
merged 2 commits into from
Mar 3, 2019
Merged

[NLL] Type check operations with pointer types #58673

merged 2 commits into from
Mar 3, 2019

Conversation

matthewjasper
Copy link
Contributor

@matthewjasper matthewjasper commented Feb 23, 2019 •
edited
Loading

It seems these were forgotten about. Moving to Rvalue::AddressOf simplifies the coercions from references, but I want this to be fixed as soon as possible.

r? @pnkfelix

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 23, 2019
@matthewjasper matthewjasper changed the title [NLL] Type check coercions to pointer types [NLL] Type check operations with pointer types Feb 24, 2019
pnkfelix
pnkfelix reviewed Feb 27, 2019
View reviewed changes
src/librustc_mir/borrow_check/nll/type_check/mod.rs
}
}

Rvalue::Ref(region, _borrow_kind, borrowed_place) => {
self.add_reborrow_constraint(location, region, borrowed_place);
}

// FIXME: These other cases have to be implemented in future PRs
Copy link
Member

Choose a reason for hiding this comment

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

🤣

@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 27, 2019

📌 Commit 19a54e8 has been approved by pnkfelix

@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 27, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Feb 27, 2019 •
edited
Loading

Important detail which might get overlooked since I don't see a bug associated with this PR: the problem here is only exposed via #![feature(nll)] (or -Z borrowck=mir). It does not affect borrowck=migrate and thus does not affect the 2018 edition stable/beta. The only people who might get impacted by this in the same manner that rustc did are those opting into #![feature(nll)] on nightly.

cc #58781

Update: My claim here was false due to flawed reasoning on my part. There are examples where AST-borrowck rejects the code but the previous version of NLL accepts it, which then causes changes like this to be hard errors under migrate mode. See for example #60927 (comment)

@bors
Copy link
Contributor

bors commented Mar 3, 2019

⌛ Testing commit 19a54e8 with merge 2cfd644...

bors added a commit that referenced this pull request Mar 3, 2019
@bors
Auto merge of #58673 - matthewjasper:typeck-ptr-coercions, r=pnkfelix
2cfd644 
[NLL] Type check operations with pointer types

It seems these were forgotten about. Moving to `Rvalue::AddressOf` simplifies the coercions from references, but I want this to be fixed as soon as possible.

r? @pnkfelix
@bors
Copy link
Contributor

bors commented Mar 3, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: pnkfelix
Pushing 2cfd644 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 3, 2019
@bors bors merged commit 19a54e8 into rust-lang:master Mar 3, 2019
@bors bors mentioned this pull request Mar 3, 2019
Closed
@matthewjasper matthewjasper deleted the typeck-ptr-coercions branch March 3, 2019 19:50
@matthewjasper matthewjasper mentioned this pull request May 17, 2019
Closed
@jonas-schievink jonas-schievink mentioned this pull request May 25, 2019
Closed
@matthewjasper matthewjasper mentioned this pull request Aug 31, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pnkfelix pnkfelix pnkfelix left review comments

Assignees

@pnkfelix pnkfelix

Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@matthewjasper @pnkfelix @bors @rust-highfive