Add new temporary lifetime feature gate and super let keyword#119043
Add new temporary lifetime feature gate and super let keyword#119043dingxiangfei2009 wants to merge 3 commits intorust-lang:masterfrom
Conversation
|
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
|
I won't have time to review this PR. |
| let var_scope = if cx.tcx.sess.at_least_rust_2024() && cx.tcx.features().new_temp_lifetime { | ||
| let scope_map = cx.tcx.body_scope_map(parent_def_id); | ||
| scope_map.var_scope_new(pat.hir_id.local_id) | ||
| } else { | ||
| region_scope_tree.var_scope(pat.hir_id.local_id) | ||
| }; |
There was a problem hiding this comment.
This pattern is now appearing everywhere. Why not include this in the var_scope method itself?
There was a problem hiding this comment.
I have given this a thought. The idea is that I would like to keep the body_scope_map behind the feature gate and leave the original scope tree implementation untouched so that both can evolve independently.
|
☔ The latest upstream changes (presumably #119258) made this pull request unmergeable. Please resolve the merge conflicts. |
dingxiangfei2009
force-pushed
the
super-let
branch
from
d3a6b64 to
f2455f7
Compare
|
This has taken #119554 into account. I have decided to enclose the match guard in an intermediate scope, because the bindings will never escape into the arm body by the current |
flip1995
left a comment
flip1995
left a comment
There was a problem hiding this comment.
I don't have any objections to the Clippy changes, as long as the reviewer for the compiler changes think that this is not a concern.
I'm not the right person to review the compiler changes though.
oli-obk
left a comment
oli-obk
left a comment
There was a problem hiding this comment.
Please add some more tests. e.g. usage of super let in consts/statics. Usage at the top level (fn main() { super let x = 5; }, ...
| let temp_lifetime = if let Some(scope_map) = self.scope_map { | ||
| scope_map.temporary_scope_new(expr.hir_id.local_id) | ||
| } else { | ||
| self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id) | ||
| }; |
There was a problem hiding this comment.
This is a recurring pattern. Please add a helper for it.
| Ok(Some(if self.token.is_keyword(kw::Let) { | ||
| self.parse_local_mk(lo, attrs, capture_semi, force_collect)? | ||
| self.parse_local_mk(lo, attrs, capture_semi, force_collect, false)? | ||
| } else if self.sess.edition.at_least_rust_2024() |
There was a problem hiding this comment.
why does this need edition gating?
oli-obk
added
S-waiting-on-author
dingxiangfei2009
force-pushed
the
super-let
branch
2 times, most recently
from
fcfefea to
3351bd6
Compare
smoke tests of the new syntax move new_temp_lifetime into query restore classic scoping rules
dingxiangfei2009
force-pushed
the
super-let
branch
from
3351bd6 to
56ac680
Compare
This comment has been minimized.
This comment has been minimized.
dingxiangfei2009
force-pushed
the
super-let
branch
from
56ac680 to
6305448
Compare
|
☔ The latest upstream changes (presumably #122947) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@dingxiangfei2009 if you can resolve the conflicts we can push this forward. Thanks |
|
@Dylan-DPC thanks for the reminder! Meanwhile I will prepare the RFC document to go with this PR. I will also reduce the scope of change that this PR implements to match the new design in the upcoming RFC file. |
|
@dingxiangfei2009 thanks for the update. Since this requires an rfc that will take a while to get merged and needs to be reïmplemented, I'm closing this pr. It will be better to create a new pr once we get to that point. Thanks for contributing. |
9 participants
cc @m-ou-se @nikomatsakis
This is a test drive with the new temporary lifetime rule. With the
#![feature(new_temp_lifetime)]gate enabled, the new rules kick in to override the old scope assignments; and you will gain access tosuper let.