[mir] Special treatment for dereferencing a borrow to a static definition

[dingxiangfei2009]

Fix #70584.

As suggested by @oli-obk in this comment, one can chase the definition of the local variable being de-referenced and check if it is a true static variable. If that is the case, validate_place will admit the promotion.

This is my first time to contribute to rustc, and I have two questions.

1. A generalization to some extent is applied to decide if the promotion is possible in the static context. In case that there are more projection operations preceding the de-referencing, validate_place recursively decent into inner projection operations. I have put thoughts into its correctness but I am not totally sure about it.
2. I have a hard time to find a good place for the test case. This patch has to do with MIR, but this test case would look out of place compared to other tests in src/test/ui/mir or src/test/ui/borrowck because it does not generate errors while others do. It is tentatively placed in src/test/ui/statics for now.

Thank you for any comments and suggestions!

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[oli-obk]

cc @rust-lang/wg-mir-opt this PR uses self.temps[local], similarly to how we treat reborrows (&*foo if foo is a reference). I believe this is preferrable to the other possible solutions, namely:

• optimizing the MIR before promotion (and thus borrowck!) to convert *FOO to BAR where const FOO: &'static T = &BAR;. While I believe this optimization is strictly correct, doing such optimizations before borrowck seems dangerous
• changing MIR building to not emit those constants. This is very hard, because when building the MIR for a BAR access, we don't know whether it's going to be for a reference or for a read.

[oli-obk]

This can just be // check-pass, as the error message used to be emitted during borrowchecking.

Please leave a comment with a link to the issue and a word or two about why this test exists

[oli-obk]

• A generalization to some extent is applied to decide if the promotion is possible in the static context. In case that there are more projection operations preceding the de-referencing, validate_place recursively decent into inner projection operations. I have put thoughts into its correctness but I am not totally sure about it.

I commented on this directly in the diff. I don't believe any problematic situations can occur today, but better safe than sorry.

• I have a hard time to find a good place for the test case. This patch has to do with MIR, but this test case would look out of place compared to other tests in src/test/ui/mir or src/test/ui/borrowck because it does not generate errors while others do. It is tentatively placed in src/test/ui/statics for now.

That's a good place. We have loads of tests that do not emit errors. Since this is about promotion it could also live in src/test/ui/consts, but as this issue is specifically about statics, let's leave the test where you put it.

[oli-obk]

@bors r+

Thanks!

[bors]

📌 Commit c511454 has been approved by oli-obk

[bors]

⌛ Testing commit c511454 with merge 18e2a89...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 18e2a89 to master...

[oli-obk]

cc @rust-lang/wg-const-eval (sorry for the late ping). This PR undoes a regression in implicit promotion within static items

[RalfJung]

Ah I guess this is fallout from making pointers-to-statics into const pointer literals? Is there anything else that this promotes now that used to not get promoted?

[oli-obk]

no, this just reverts that regression, it doesn't promote more things than were promoted before the regression was introduced