Skip to content

Do not ignore lifetime bounds in Copy impls#47877

Merged
bors merged 1 commit intorust-lang:masterfrom
spastorino:lifetime-bounds-in-copy
Feb 5, 2018
Merged

Do not ignore lifetime bounds in Copy impls#47877
bors merged 1 commit intorust-lang:masterfrom
spastorino:lifetime-bounds-in-copy

Conversation

@spastorino
Copy link
Member

@spastorino spastorino commented Jan 30, 2018 •
edited by nikomatsakis
Loading

nikomatsakis
nikomatsakis reviewed Jan 30, 2018
View reviewed changes
src/librustc_mir/borrow_check/nll/type_check/mod.rs Outdated
Copy link
Contributor

@nikomatsakis nikomatsakis Jan 30, 2018

Choose a reason for hiding this comment

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

Let's add a comment here:

In order to have a Copy operand, the type T of the value must be Copy. Note that we prove that T: Copy, rather than using the type_moves_by_default test. This is important because type_moves_by_default ignores the resulting region obligations and assumes they pass. This can result in bounds from Copy impls being unsoundly ignored (e.g., #29149). Note that we decide to use Copy before knowing whether the bounds fully apply: in effect, the rule is that if a value of some type could implement Copy, then it must.

nikomatsakis
nikomatsakis requested changes Jan 30, 2018
View reviewed changes
src/test/compile-fail/nll/do-not-ignore-lifetime-bounds-in-copy.rs Outdated
Copy link
Contributor

@nikomatsakis nikomatsakis Jan 30, 2018

Choose a reason for hiding this comment

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

Can you add a comment that mentions #29149 ? You could basically copy and paste the comment I described above.

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 30, 2018
@spastorino spastorino force-pushed the lifetime-bounds-in-copy branch 2 times, most recently from 59d11b0 to dbe7bda Compare January 30, 2018 14:21
@spastorino
Copy link
Member Author

spastorino commented Jan 30, 2018

@nikomatsakis Comments added!

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jan 30, 2018

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 30, 2018

📌 Commit dbe7bda has been approved by nikomatsakis

This was referenced Jan 30, 2018
Closed
Closed
@spastorino spastorino force-pushed the lifetime-bounds-in-copy branch from dbe7bda to b9f7564 Compare January 30, 2018 17:01
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jan 30, 2018

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 30, 2018

📌 Commit b9f7564 has been approved by nikomatsakis

@kennytm kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Feb 3, 2018
@kennytm
Rollup merge of rust-lang#47877 - spastorino:lifetime-bounds-in-copy,…
c2d590f 
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
@kennytm kennytm mentioned this pull request Feb 3, 2018
Closed
bors added a commit that referenced this pull request Feb 3, 2018
@bors
Auto merge of #47984 - kennytm:rollup, r=kennytm
6f6ac5e 
Rollup of 9 pull requests

- Successful merges: #47753, #47862, #47877, #47896, #47912, #47944, #47947, #47978, #47958
- Failed merges:
kennytm added a commit to kennytm/rust that referenced this pull request Feb 4, 2018
@kennytm
Rollup merge of rust-lang#47877 - spastorino:lifetime-bounds-in-copy,…
dd25825 
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
@kennytm kennytm mentioned this pull request Feb 4, 2018
Closed
kennytm added a commit to kennytm/rust that referenced this pull request Feb 4, 2018
@kennytm
Rollup merge of rust-lang#47877 - spastorino:lifetime-bounds-in-copy,…
f3dc756 
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
@kennytm kennytm mentioned this pull request Feb 4, 2018
Merged
bors added a commit that referenced this pull request Feb 4, 2018
@bors
Auto merge of #47998 - kennytm:rollup, r=kennytm
7dd3864 
Rollup of 10 pull requests

- Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892
- Failed merges:
bors added a commit that referenced this pull request Feb 4, 2018
@bors
Auto merge of #47998 - kennytm:rollup, r=kennytm
e7e982a 
Rollup of 10 pull requests

- Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892
- Failed merges:
@bors bors merged commit b9f7564 into rust-lang:master Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels

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

@spastorino @nikomatsakis @bors @kennytm