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

illegal moves permitted in statics and constants #38520

Closed
nikomatsakis opened this issue Dec 21, 2016 · 8 comments · Fixed by #39927
Closed

illegal moves permitted in statics and constants #38520

nikomatsakis opened this issue Dec 21, 2016 · 8 comments · Fixed by #39927
Labels
A-borrow-checker Area: The borrow checker I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Dec 21, 2016

This code compiles on nightly today but it should not, because Foo is not Copy:

#![feature(const_fn)]

struct Foo(usize);

const fn get(x: Foo) -> usize {
    x.0
}

const X: Foo = Foo(22);
static Y: usize = get(*&X); // also a problem for `const Y`

fn main() {
}

Note that the get(*&X) in a function body properly results in an error.

@nikomatsakis nikomatsakis added A-borrow-checker Area: The borrow checker I-nominated I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 21, 2016
@nikomatsakis
Copy link
Contributor Author

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Dec 22, 2016
@nikomatsakis
Copy link
Contributor Author

This is likely a problem of how borrowck kind of special-cases the handling of statics a bit. Should be relatively easy to fix. Could be fixed "en passante" by MIR borrowck; I'll also try to add some mentoring tips.

@cengiz-io
Copy link
Contributor

Hello @nikomatsakis. I'd like to take a shot at this if I'm allowed.

@cengiz-io
Copy link
Contributor

cengiz-io commented Dec 26, 2016

My first observation is that this isn't related to const_fn. Am I right?

I'm reading the RFC now and doesn't seem to require Copy on const_fns.

EDIT: I somehow missed the static Y: usize = get(*&X); part. Now it's starting to make sense 👍

@cengiz-io
Copy link
Contributor

Hello again @nikomatsakis

I'm working on this right now.

@nikomatsakis
Copy link
Contributor Author

@cengizio ok -- how are you making out? Sorry I didn't get around to leaving any notes on how to proceed. Let me know if you still want me to go digging around -- or privmsg me on IRC (nmatsakis).

@nikomatsakis
Copy link
Contributor Author

@cengizio so looking at the code a bit, it seems clear that the problem is that the borrowck code has this kind of custom and woefully insufficient bit of code dedicated to statics and constants. It really ought to be using the same pathways as the rest of the code. (In particular, the ExprUseVisitor, which is a bit of code that interprets the HIR and gives callbacks corresponding to moves, borrows, etc). Unfortunately, those pathways are kind of setup more for regular expression bodies; I'm not 100% sure how easy they will be to apply.

I feel like this bug fix is "just" a refactoring, but the refactoring itself may not be all that simple. Doing the borrowck on the MIR we generate for constants would fix it, but it's not clear when MIR borrowck will land.

@cengiz-io
Copy link
Contributor

@nikomatsakis as we talked in IRC, I think this should be discussed widely before we start implementing anything.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Feb 19, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 24, 2017
…k-2, r=eddyb

transition borrowck to visit all **bodies** and not item-likes

This is a better structure for incremental compilation and also more compatible with the eventual borrowck mir. It also fixes rust-lang#38520 as a drive-by fix.

r? @eddyb
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Feb 28, 2017
bors added a commit that referenced this issue Mar 3, 2017
transition borrowck to visit all **bodies** and not item-likes

This is a better structure for incremental compilation and also more compatible with the eventual borrowck mir. It also fixes #38520 as a drive-by fix.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants