Adjust cannot move out of static error in prep for removing Statics from Place

[spastorino]

This is in preparation to remove Statics from Place. We are going to loose some information on Mir so we need to make these static errors a bit more generic.

r? @oli-obk

[oli-obk]

cc @rust-lang/wg-diagnostics By using a more general error message, we can in the future treat reading from a static BAR as *FOO where FOO is a compilergenerated constant of value &BAR. This unifies all borrowckecking and similiar logic that is duplicated between reading from immutable references and reading from immutable statics. We want to prepare as much diagnostics work before actually doing the PR so the PR is very simple to review without much noise

[matthewjasper]

How is this unifying anything? This is replacing one special-cased error message with a worse special-cased error message.

[spastorino]

@matthewjasper going to provide another PR that changes the cannot move out of xxx because it is borrowed into cannot move out of an immutable place and that's where the unification shows up. I can add an extra commit to this PR with that, I think that would be more clear. Seemed to me that @oli-obk wanted everything in separate PRs.

[matthewjasper]

That sounds even worse. That would be regressing diagnostics across 2 PRs so that a third PR can be slightly smaller?

[spastorino]

@matthewjasper yeah I've said that because those are the unified errors, the ones about statics and the ones about borrows. I guess I understand what you meant, the errors are going to regress but it's the drawback of the idea that @oli-obk explained above.

I could provide one PR for each error, one PR for all of them or one PR for all the work. It doesn't change much to me, @oli-obk wanted me to split things for easier review. I'd do whatever you and @oli-obk prefer, it's the same for me.

@matthewjasper just in case I did get something wrong, are you against the Static change because of diagnostics are going to regress? or is it that you just prefer everything in one PR?.

[matthewjasper]

I definitely think removing Places based on statics is a good change. I don't see how this PR helps with that though. The diagnostic regression from only making the change to having statics behind references isn't as bad as the one proposed here IMO.

I also think that it would be possible to avoid the regression entirely without that much work. I can try to have a PR that moves all static accesses to always go through an Rvalue::Ref without changing the diagnostics up this week if that's something that you would be interested in.

[bors]

☔ The latest upstream changes (presumably #66170) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

I can try to have a PR that moves all static accesses to always go through an Rvalue::Ref without changing the diagnostics up this week if that's something that you would be interested in.

Huh, interesting. Can you elaborate on how you think this could be done? Add a new BorrowKind so the Rvalue::Ref knows about the static?

[spastorino]

The diagnostic regression from only making the change to having statics behind references isn't as bad as the one proposed here IMO.

So:
current message: cannot move out of static item BAR
message after static change: cannot move out of a shared reference
message after this PR: cannot move out of an immutable place

I don't know if I understood what you meant in your previous cited sentence. Did you mean that the diagnostic message after static change is better than message after this PR?.
We were considering that is not and that's why this PR exists.

I also think that it would be possible to avoid the regression entirely without that much work. I can try to have a PR that moves all static accesses to always go through an Rvalue::Ref without changing the diagnostics up this week if that's something that you would be interested in.

I think this changes everything 😄, I have the same question @oli-obk asked, can you elaborate more?. I could also tackle this as part of the bigger change.

[matthewjasper]

The plan would be to move the information currently in BindingForm out of LocalDecl and probably also out of Body. This is something that I've wanted to do for a while now but have been focusing on other things. The mapping from the new temporaries to the statics that they refer to would then be added to that.

Thinking about this some more I'll do (enough of) the BindingInfo refactoring in it's own PR and then write notes on how to handle the change to by-ref statics.

[eddyb]

I don't think the "shared" part of "shared reference" or "immutable" part of "immutable place" are relevant.

The important thing is that you don't own the place, be it a dereference or a static.

[JohnCSimon]

Ping from triage
@spastorino Can you please address the merge conflicts?
Thank you!

[matthewjasper]

This is no longer needed.