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

Qualify constness in rvalue expressions for promotion to globals. #21744

Merged
merged 15 commits into from
Feb 16, 2015

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jan 29, 2015

This includes everything necessary for promoting borrows of constant rvalues to 'static.
That is, &expr will have the type &'static T if const T: &'static T = &expr; is valid.
There is a small exception, dereferences of raw pointers, as they misbehave.
They still "work" in constants as I didn't want to break legitimate uses (are there any?).

The qualification done here can be expanded to allow simple CTFE via const fn.

@eddyb
Copy link
Member Author

eddyb commented Jan 29, 2015

@bors try acb37ce

@rust-highfive
Copy link
Collaborator

r? @nick29581

(rust_highfive has picked a reviewer for you, use r? to override)

@bors
Copy link
Contributor

bors commented Jan 29, 2015

⌛ Testing commit acb37ce with merge f3f47ad...

@eddyb
Copy link
Member Author

eddyb commented Jan 29, 2015

r? @nikomatsakis

@bors
Copy link
Contributor

bors commented Jan 29, 2015

💔 Test failed - auto-win-32-nopt-t

@eddyb
Copy link
Member Author

eddyb commented Jan 30, 2015

@bors try 91d51c8

@bors
Copy link
Contributor

bors commented Jan 30, 2015

⌛ Testing commit 91d51c8 with merge c41f7b6...

@bors
Copy link
Contributor

bors commented Jan 30, 2015

💔 Test failed - auto-mac-64-opt

@eddyb
Copy link
Member Author

eddyb commented Jan 30, 2015

@bors try f9556d8

@bors
Copy link
Contributor

bors commented Jan 30, 2015

⌛ Testing commit f9556d8 with merge 70b6649...

@bors
Copy link
Contributor

bors commented Jan 30, 2015

💔 Test failed - auto-mac-64-nopt-t

@nikomatsakis
Copy link
Contributor

This looks great: r+

I have some nits and minor comments, which you can find as comments in this commit: nikomatsakis@feb8ca7

@eddyb
Copy link
Member Author

eddyb commented Feb 6, 2015

Blocked on #21996.

@eddyb
Copy link
Member Author

eddyb commented Feb 11, 2015

@bors r=nikomatsakis a091c5a

@bors
Copy link
Contributor

bors commented Feb 11, 2015

⌛ Testing commit a091c5a with merge 5fcfa12...

@bors
Copy link
Contributor

bors commented Feb 12, 2015

💔 Test failed - auto-mac-32-opt

@bors
Copy link
Contributor

bors commented Feb 16, 2015

💔 Test failed - auto-linux-64-nopt-t

@eddyb
Copy link
Member Author

eddyb commented Feb 16, 2015

@bors r=nikomatsakis b49f528

@bors
Copy link
Contributor

bors commented Feb 16, 2015

⌛ Testing commit b49f528 with merge e4e7aa2...

bors added a commit that referenced this pull request Feb 16, 2015
This includes everything necessary for promoting borrows of constant rvalues to `'static`.
That is, `&expr` will have the type `&'static T` if `const T: &'static T = &expr;` is valid.
There is a small exception, dereferences of raw pointers, as they misbehave.
They still "work" in constants as I didn't want to break legitimate uses (are there any?).

The qualification done here can be expanded to allow simple CTFE via `const fn`.
@bors bors merged commit b49f528 into rust-lang:master Feb 16, 2015
@eddyb eddyb deleted the rvalue-promotion branch February 16, 2015 19:07
@huonw
Copy link
Member

huonw commented Feb 17, 2015

Isn't this designed to make let x: &'static i32 = &1; work? Shouldn't there be tests for that sort of behaviour?

@brson
Copy link
Contributor

brson commented Feb 17, 2015

Epic fight with @bors!

@huonw
Copy link
Member

huonw commented Feb 17, 2015

(If tests are added, they can close rust-lang/rfcs#827.)

@eddyb
Copy link
Member Author

eddyb commented Feb 17, 2015

@huonw No, that part needs an RFC. @nikomatsakis wanted to write one for the longest time - in any case, there's a few different directions to go from this base implementation.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 7, 2020
Do not promote &mut of a non-ZST ever

Since ~pre-1.0~ 1.36, we have accepted code like this:
```rust
static mut TEST: &'static mut [i32] = {
    let x = &mut [1,2,3];
    x
};
```
I tracked it back to rust-lang#21744, but unfortunately could not find any discussion or RFC that would explain why we thought this was a good idea. And it's not, it breaks all sorts of things -- see rust-lang#75556.

To fix rust-lang#75556, we have to stop promoting non-ZST mutable references no matter the context, which is what this PR does. It's a breaking change.

Notice that this still works, since it does not rely on promotion:
```rust
static mut TEST: &'static mut [i32] = &mut [0,1,2];
```

Cc @rust-lang/wg-const-eval
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2020
Do not promote &mut of a non-ZST ever

Since ~pre-1.0~ 1.36, we have accepted code like this:
```rust
static mut TEST: &'static mut [i32] = {
    let x = &mut [1,2,3];
    x
};
```
I tracked it back to rust-lang#21744, but unfortunately could not find any discussion or RFC that would explain why we thought this was a good idea. And it's not, it breaks all sorts of things -- see rust-lang#75556.

To fix rust-lang#75556, we have to stop promoting non-ZST mutable references no matter the context, which is what this PR does. It's a breaking change.

Notice that this still works, since it does not rely on promotion:
```rust
static mut TEST: &'static mut [i32] = &mut [0,1,2];
```

Cc `@rust-lang/wg-const-eval`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants