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 ignore lifetime bounds in Copy impls #47877

Merged
merged 1 commit into from
Feb 5, 2018

Conversation

spastorino
Copy link
Member

@spastorino spastorino commented Jan 30, 2018

substs: tcx.mk_substs_trait(place_ty.to_ty(tcx), &[]),
};

self.cx.prove_trait_ref(trait_ref, location);
Copy link
Contributor

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.

// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(nll)]
Copy link
Contributor

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

@nikomatsakis Comments added!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 30, 2018

📌 Commit dbe7bda has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

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
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 3, 2018
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
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
kennytm added a commit to kennytm/rust that referenced this pull request Feb 4, 2018
… r=nikomatsakis

Do not ignore lifetime bounds in Copy impls

cc rust-lang#29149

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 4, 2018
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
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
Labels
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.

4 participants