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

Stop generating an intermediate deref when taking references to statics #77020

Closed
wants to merge 2 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Sep 21, 2020

r? @RalfJung @ecstatic-morse

So... the motivation for this change was to make things less fragile. There's still work to be done if we really want to go all the way to having more Rvalues for different ways to interact with statics and fully stop relying on metadata of locals for soundness.

fixes #54366

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ `&mut` is only allowed in `const fn`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ modifying a static's initial value from another static's initializer
Copy link
Contributor

Choose a reason for hiding this comment

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

This is bad. We no longer catch this pre-monomorphization even though it has&mut. You'll need to handle this case somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but I see no way to distinguish this from the &mut STDERR_BUFFER_SPACE that we'd see for just let x = STDERR_BUFFER_SPACE without going all the way to having custom Rvalues for that

Copy link
Contributor

Choose a reason for hiding this comment

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

There's always the HIR const-checker I guess 😆. I haven't got around to removing it yet.

As written, this PR allows stuff like let x = &mut STATIC_MUT in a const-context on stable, so you'll need to handle it somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately

struct Foo {
    field: *mut i32
}

static mut FOO: Foo = Foo {
    field: &mut [42] as *mut [i32] as *mut i32,
};

is stable, so I can't just ban mutable references.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But yea, I gottaa figure something out how to not expose it even more...

@RalfJung
Copy link
Member

fixes #54366

That's a very different issue that the last PR with a similar title (#76982)? I am somewhat confused. I assume this is supposed to resolve #74840?

Also could you describe at a high level what the approach to solving #74840 is, compared to #76982? That would be easier than trying to reverse engineer that from the PR.^^

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 22, 2020

I'm trying to get rid of our reliance on the local_info for the soundness of accessing static items. The fact that this now fixes #54366 is a total unintended side effect that I only noticed because the canary test for it now fails.

This is not related to #74840 anymore, as that issue is now solely about making uninhabited extern types forbidden via a future incompat lint.

This PR takes &FOO expressions and directly converts them to constants of pointer type instead of having &*&FOO where the trailing &FOO is a pointer type. Though that causes us to stop being able to distinguish the user writing let x = FOO; and the user writing let x = *&FOO;, causing the former to now fail to compile in case FOO is a mutable static. So as it stands, this PR cannot work

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 22, 2020

I have explored the design space around Rvalue::ReadStatic and similar locally and have come to the conclusion that they are not feasible since they mostly just amount to more complexity without any reduction.

We're in a fairly good sweet spot with these &*&STATIC desugarings.

That said, we can still do the active PR once mutable references and dereferencing becomes legal in statics

@oli-obk oli-obk closed this Sep 22, 2020
@RalfJung
Copy link
Member

Okay, I opened #77096 for the problems the current static handling is causing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[thread_local] static mut is allowed to have 'static lifetime
4 participants