Skip to content

Commit 27c6e40

Browse files
authored
Rollup merge of #139112 - m-ou-se:super-let, r=lcnr
Implement `super let` Tracking issue: #139076 This implements `super let` as proposed in #139080, based on the following two equivalence rules. 1. For all expressions `$expr` in any context, these are equivalent: - `& $expr` - `{ super let a = & $expr; a }` 2. And, additionally, these are equivalent in any context when `$expr` is a temporary (aka rvalue): - `& $expr` - `{ super let a = $expr; & a }` So far, this experiment has a few interesting results: ## Interesting result 1 In this snippet: ```rust super let a = f(&temp()); ``` I originally expected temporary `temp()` would be dropped at the end of the statement (`;`), just like in a regular `let`, because `temp()` is not subject to temporary lifetime extension. However, it turns out that that would break the fundamental equivalence rules. For example, in ```rust g(&f(&temp())); ``` the temporary `temp()` will be dropped at the `;`. The first equivalence rule tells us this must be equivalent: ```rust g({ super let a = &f(&temp()); a }); ``` But that means that `temp()` must live until the last `;` (after `g()`), not just the first `;` (after `f()`). While this was somewhat surprising to me at first, it does match the exact behavior we need for `pin!()`: The following _should work_. (See also #138718) ```rust g(pin!(f(&mut temp()))); ``` Here, `temp()` lives until the end of the statement. This makes sense from the perspective of the user, as no other `;` or `{}` are visible. Whether `pin!()` uses a `{}` block internally or not should be irrelevant. This means that _nothing_ in a `super let` statement will be dropped at the end of that super let statement. It does not even need its own scope. This raises questions that are useful for later on: - Will this make temporaries live _too long_ in cases where `super let` is used not in a hidden block in a macro, but as a visible statement in code like the following? ```rust let writer = { super let file = File::create(&format!("/home/{user}/test")); Writer::new(&file) }; ``` - Is a `let` statement in a block still the right syntax for this? Considering it has _no_ scope of its own, maybe neither a block nor a statement should be involved This leads me to think that instead of `{ super let $pat = $init; $expr }`, we might want to consider something like `let $pat = $init in $expr` or `$expr where $pat = $init`. Although there are also issues with these, as it isn't obvious anymore if `$init` should be subject to temporary lifetime extension. (Do we want both `let _ = _ in ..` and `super let _ = _ in ..`?) ## Interesting result 2 What about `super let x;` without initializer? ```rust let a = { super let x; x = temp(); &x }; ``` This works fine with the implementation in this PR: `x` is extended to live as long as `a`. While it matches my expectations, a somewhat interesting thing to realize is that these are _not_ equivalent: - `super let x = $expr;` - `super let x; x = $expr;` In the first case, all temporaries in $expr will live at least as long as (the result of) the surrounding block. In the second case, temporaries will be dropped at the end of the assignment statement. (Because the assignment statement itself "is not `super`".) This difference in behavior might be confusing, but it _might_ be useful. One might want to extend the lifetime of a variable without extending all the temporaries in the initializer expression. On the other hand, that can also be expressed as: - `let x = $expr; super let x = x;` (w/o temporary lifetime extension), or - `super let x = { $expr };` (w/ temporary lifetime extension) So, this raises these questions: - Do we want to accept `super let x;` without initializer at all? - Does it make sense for statements other than let statements to be "super"? An expression statement also drops temporaries at its `;`, so now that we discovered that `super let` basically disables that `;` (see interesting result 1), is there a use to having other statements without their own scope? (I don't think that's ever useful?) ## Interesting result 3 This works now: ```rust super let Some(x) = a.get(i) else { return }; ``` I didn't put in any special cases for `super let else`. This is just the behavior that 'naturally' falls out when implementing `super let` without thinking of the `let else` case. - Should `super let else` work? ## Interesting result 4 This 'works': ```rust fn main() { super let a = 123; } ``` I didn't put in any special cases for `super let` at function scope. I had expected the code to cause an ICE or other weird failure when used at function body scope, because there's no way to let the variable live as long as the result of the function. This raises the question: - Does this mean that this behavior is the natural/expected behavior when `super let` is used at function scope? Or is this just a quirk and should we explicitly disallow `super let` in a function body? (Probably the latter.) --- The questions above do not need an answer to land this PR. These questions should be considered when redesigning/rfc'ing/stabilizing the feature.
2 parents 9955b76 + ccbef74 commit 27c6e40

File tree

18 files changed

+615
-50
lines changed

18 files changed

+615
-50
lines changed

compiler/rustc_ast/src/ast.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,7 @@ pub enum MacStmtStyle {
11751175
#[derive(Clone, Encodable, Decodable, Debug)]
11761176
pub struct Local {
11771177
pub id: NodeId,
1178+
pub super_: Option<Span>,
11781179
pub pat: P<Pat>,
11791180
pub ty: Option<P<Ty>>,
11801181
pub kind: LocalKind,
@@ -3932,7 +3933,7 @@ mod size_asserts {
39323933
static_assert_size!(Item, 144);
39333934
static_assert_size!(ItemKind, 80);
39343935
static_assert_size!(LitKind, 24);
3935-
static_assert_size!(Local, 80);
3936+
static_assert_size!(Local, 96);
39363937
static_assert_size!(MetaItemLit, 40);
39373938
static_assert_size!(Param, 40);
39383939
static_assert_size!(Pat, 72);

compiler/rustc_ast/src/mut_visit.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,8 @@ fn walk_parenthesized_parameter_data<T: MutVisitor>(vis: &mut T, args: &mut Pare
704704
}
705705

706706
fn walk_local<T: MutVisitor>(vis: &mut T, local: &mut P<Local>) {
707-
let Local { id, pat, ty, kind, span, colon_sp, attrs, tokens } = local.deref_mut();
707+
let Local { id, super_, pat, ty, kind, span, colon_sp, attrs, tokens } = local.deref_mut();
708+
visit_opt(super_, |sp| vis.visit_span(sp));
708709
vis.visit_id(id);
709710
visit_attrs(vis, attrs);
710711
vis.visit_pat(pat);

compiler/rustc_ast/src/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ pub fn walk_crate<'a, V: Visitor<'a>>(visitor: &mut V, krate: &'a Crate) -> V::R
323323
}
324324

325325
pub fn walk_local<'a, V: Visitor<'a>>(visitor: &mut V, local: &'a Local) -> V::Result {
326-
let Local { id: _, pat, ty, kind, span: _, colon_sp: _, attrs, tokens: _ } = local;
326+
let Local { id: _, super_: _, pat, ty, kind, span: _, colon_sp: _, attrs, tokens: _ } = local;
327327
walk_list!(visitor, visit_attribute, attrs);
328328
try_visit!(visitor.visit_pat(pat));
329329
visit_opt!(visitor, visit_ty, ty);

compiler/rustc_ast_lowering/src/block.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
9595

9696
fn lower_local(&mut self, l: &Local) -> &'hir hir::LetStmt<'hir> {
9797
// Let statements are allowed to have impl trait in bindings.
98+
let super_ = l.super_;
9899
let ty = l.ty.as_ref().map(|t| {
99100
self.lower_ty(t, self.impl_trait_in_bindings_ctxt(ImplTraitPosition::Variable))
100101
});
@@ -109,7 +110,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
109110
let span = self.lower_span(l.span);
110111
let source = hir::LocalSource::Normal;
111112
self.lower_attrs(hir_id, &l.attrs, l.span);
112-
self.arena.alloc(hir::LetStmt { hir_id, ty, pat, init, els, span, source })
113+
self.arena.alloc(hir::LetStmt { hir_id, super_, ty, pat, init, els, span, source })
113114
}
114115

115116
fn lower_block_check_mode(&mut self, b: &BlockCheckMode) -> hir::BlockCheckMode {

compiler/rustc_ast_lowering/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2218,6 +2218,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
22182218
self.attrs.insert(hir_id.local_id, a);
22192219
}
22202220
let local = hir::LetStmt {
2221+
super_: None,
22212222
hir_id,
22222223
init,
22232224
pat,

compiler/rustc_ast_pretty/src/pprust/state.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,9 @@ impl<'a> State<'a> {
13361336
self.print_outer_attributes(&loc.attrs);
13371337
self.space_if_not_bol();
13381338
self.ibox(INDENT_UNIT);
1339+
if loc.super_.is_some() {
1340+
self.word_nbsp("super");
1341+
}
13391342
self.word_nbsp("let");
13401343

13411344
self.ibox(INDENT_UNIT);

compiler/rustc_expand/src/build.rs

+2
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ impl<'a> ExtCtxt<'a> {
230230
self.pat_ident(sp, ident)
231231
};
232232
let local = P(ast::Local {
233+
super_: None,
233234
pat,
234235
ty,
235236
id: ast::DUMMY_NODE_ID,
@@ -245,6 +246,7 @@ impl<'a> ExtCtxt<'a> {
245246
/// Generates `let _: Type;`, which is usually used for type assertions.
246247
pub fn stmt_let_type_only(&self, span: Span, ty: P<ast::Ty>) -> ast::Stmt {
247248
let local = P(ast::Local {
249+
super_: None,
248250
pat: self.pat_wild(span),
249251
ty: Some(ty),
250252
id: ast::DUMMY_NODE_ID,

compiler/rustc_feature/src/unstable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ declare_features! (
630630
/// Allows string patterns to dereference values to match them.
631631
(unstable, string_deref_patterns, "1.67.0", Some(87121)),
632632
/// Allows `super let` statements.
633-
(incomplete, super_let, "CURRENT_RUSTC_VERSION", Some(139076)),
633+
(unstable, super_let, "CURRENT_RUSTC_VERSION", Some(139076)),
634634
/// Allows subtrait items to shadow supertrait items.
635635
(unstable, supertrait_item_shadowing, "1.86.0", Some(89151)),
636636
/// Allows using `#[thread_local]` on `static` items.

compiler/rustc_hir/src/hir.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1821,6 +1821,8 @@ pub enum StmtKind<'hir> {
18211821
/// Represents a `let` statement (i.e., `let <pat>:<ty> = <init>;`).
18221822
#[derive(Debug, Clone, Copy, HashStable_Generic)]
18231823
pub struct LetStmt<'hir> {
1824+
/// Span of `super` in `super let`.
1825+
pub super_: Option<Span>,
18241826
pub pat: &'hir Pat<'hir>,
18251827
/// Type annotation, if any (otherwise the type will be inferred).
18261828
pub ty: Option<&'hir Ty<'hir>>,
@@ -4854,7 +4856,7 @@ mod size_asserts {
48544856
static_assert_size!(ImplItemKind<'_>, 40);
48554857
static_assert_size!(Item<'_>, 88);
48564858
static_assert_size!(ItemKind<'_>, 64);
4857-
static_assert_size!(LetStmt<'_>, 64);
4859+
static_assert_size!(LetStmt<'_>, 72);
48584860
static_assert_size!(Param<'_>, 32);
48594861
static_assert_size!(Pat<'_>, 72);
48604862
static_assert_size!(Path<'_>, 40);

compiler/rustc_hir_analysis/src/check/region.rs

+85-16
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
99
use std::mem;
1010

11+
use rustc_data_structures::fx::FxHashMap;
1112
use rustc_hir as hir;
1213
use rustc_hir::def_id::DefId;
1314
use rustc_hir::intravisit::{self, Visitor};
@@ -44,6 +45,8 @@ struct ScopeResolutionVisitor<'tcx> {
4445
scope_tree: ScopeTree,
4546

4647
cx: Context,
48+
49+
extended_super_lets: FxHashMap<hir::ItemLocalId, Option<Scope>>,
4750
}
4851

4952
/// Records the lifetime of a local variable as `cx.var_parent`
@@ -214,18 +217,29 @@ fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hi
214217
let stmt_id = stmt.hir_id.local_id;
215218
debug!("resolve_stmt(stmt.id={:?})", stmt_id);
216219

217-
// Every statement will clean up the temporaries created during
218-
// execution of that statement. Therefore each statement has an
219-
// associated destruction scope that represents the scope of the
220-
// statement plus its destructors, and thus the scope for which
221-
// regions referenced by the destructors need to survive.
220+
if let hir::StmtKind::Let(LetStmt { super_: Some(_), .. }) = stmt.kind {
221+
// `super let` statement does not start a new scope, such that
222+
//
223+
// { super let x = identity(&temp()); &x }.method();
224+
//
225+
// behaves exactly as
226+
//
227+
// (&identity(&temp()).method();
228+
intravisit::walk_stmt(visitor, stmt);
229+
} else {
230+
// Every statement will clean up the temporaries created during
231+
// execution of that statement. Therefore each statement has an
232+
// associated destruction scope that represents the scope of the
233+
// statement plus its destructors, and thus the scope for which
234+
// regions referenced by the destructors need to survive.
222235

223-
let prev_parent = visitor.cx.parent;
224-
visitor.enter_node_scope_with_dtor(stmt_id, true);
236+
let prev_parent = visitor.cx.parent;
237+
visitor.enter_node_scope_with_dtor(stmt_id, true);
225238

226-
intravisit::walk_stmt(visitor, stmt);
239+
intravisit::walk_stmt(visitor, stmt);
227240

228-
visitor.cx.parent = prev_parent;
241+
visitor.cx.parent = prev_parent;
242+
}
229243
}
230244

231245
fn resolve_expr<'tcx>(
@@ -478,14 +492,19 @@ fn resolve_expr<'tcx>(
478492
visitor.cx = prev_cx;
479493
}
480494

495+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
496+
enum LetKind {
497+
Regular,
498+
Super,
499+
}
500+
481501
fn resolve_local<'tcx>(
482502
visitor: &mut ScopeResolutionVisitor<'tcx>,
483503
pat: Option<&'tcx hir::Pat<'tcx>>,
484504
init: Option<&'tcx hir::Expr<'tcx>>,
505+
let_kind: LetKind,
485506
) {
486-
debug!("resolve_local(pat={:?}, init={:?})", pat, init);
487-
488-
let blk_scope = visitor.cx.var_parent;
507+
debug!("resolve_local(pat={:?}, init={:?}, let_kind={:?})", pat, init, let_kind);
489508

490509
// As an exception to the normal rules governing temporary
491510
// lifetimes, initializers in a let have a temporary lifetime
@@ -543,14 +562,50 @@ fn resolve_local<'tcx>(
543562
// A, but the inner rvalues `a()` and `b()` have an extended lifetime
544563
// due to rule C.
545564

565+
if let_kind == LetKind::Super {
566+
if let Some(scope) = visitor.extended_super_lets.remove(&pat.unwrap().hir_id.local_id) {
567+
// This expression was lifetime-extended by a parent let binding. E.g.
568+
//
569+
// let a = {
570+
// super let b = temp();
571+
// &b
572+
// };
573+
//
574+
// (Which needs to behave exactly as: let a = &temp();)
575+
//
576+
// Processing of `let a` will have already decided to extend the lifetime of this
577+
// `super let` to its own var_scope. We use that scope.
578+
visitor.cx.var_parent = scope;
579+
} else {
580+
// This `super let` is not subject to lifetime extension from a parent let binding. E.g.
581+
//
582+
// identity({ super let x = temp(); &x }).method();
583+
//
584+
// (Which needs to behave exactly as: identity(&temp()).method();)
585+
//
586+
// Iterate up to the enclosing destruction scope to find the same scope that will also
587+
// be used for the result of the block itself.
588+
while let Some(s) = visitor.cx.var_parent {
589+
let parent = visitor.scope_tree.parent_map.get(&s).cloned();
590+
if let Some(Scope { data: ScopeData::Destruction, .. }) = parent {
591+
break;
592+
}
593+
visitor.cx.var_parent = parent;
594+
}
595+
}
596+
}
597+
546598
if let Some(expr) = init {
547-
record_rvalue_scope_if_borrow_expr(visitor, expr, blk_scope);
599+
record_rvalue_scope_if_borrow_expr(visitor, expr, visitor.cx.var_parent);
548600

549601
if let Some(pat) = pat {
550602
if is_binding_pat(pat) {
551603
visitor.scope_tree.record_rvalue_candidate(
552604
expr.hir_id,
553-
RvalueCandidate { target: expr.hir_id.local_id, lifetime: blk_scope },
605+
RvalueCandidate {
606+
target: expr.hir_id.local_id,
607+
lifetime: visitor.cx.var_parent,
608+
},
554609
);
555610
}
556611
}
@@ -562,6 +617,7 @@ fn resolve_local<'tcx>(
562617
if let Some(expr) = init {
563618
visitor.visit_expr(expr);
564619
}
620+
565621
if let Some(pat) = pat {
566622
visitor.visit_pat(pat);
567623
}
@@ -640,6 +696,7 @@ fn resolve_local<'tcx>(
640696
/// | [ ..., E&, ... ]
641697
/// | ( ..., E&, ... )
642698
/// | {...; E&}
699+
/// | { super let ... = E&; ... }
643700
/// | if _ { ...; E& } else { ...; E& }
644701
/// | match _ { ..., _ => E&, ... }
645702
/// | box E&
@@ -676,6 +733,13 @@ fn resolve_local<'tcx>(
676733
if let Some(subexpr) = block.expr {
677734
record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
678735
}
736+
for stmt in block.stmts {
737+
if let hir::StmtKind::Let(local) = stmt.kind
738+
&& let Some(_) = local.super_
739+
{
740+
visitor.extended_super_lets.insert(local.pat.hir_id.local_id, blk_id);
741+
}
742+
}
679743
}
680744
hir::ExprKind::If(_, then_block, else_block) => {
681745
record_rvalue_scope_if_borrow_expr(visitor, then_block, blk_id);
@@ -801,7 +865,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
801865
local_id: body.value.hir_id.local_id,
802866
data: ScopeData::Destruction,
803867
});
804-
resolve_local(this, None, Some(body.value));
868+
resolve_local(this, None, Some(body.value), LetKind::Regular);
805869
}
806870
})
807871
}
@@ -819,7 +883,11 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
819883
resolve_expr(self, ex, false);
820884
}
821885
fn visit_local(&mut self, l: &'tcx LetStmt<'tcx>) {
822-
resolve_local(self, Some(l.pat), l.init)
886+
let let_kind = match l.super_ {
887+
Some(_) => LetKind::Super,
888+
None => LetKind::Regular,
889+
};
890+
resolve_local(self, Some(l.pat), l.init, let_kind);
823891
}
824892
fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
825893
let body = self.tcx.hir_body(c.body);
@@ -848,6 +916,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
848916
cx: Context { parent: None, var_parent: None },
849917
pessimistic_yield: false,
850918
fixup_scopes: vec![],
919+
extended_super_lets: Default::default(),
851920
};
852921

853922
visitor.scope_tree.root_body = Some(body.value.hir_id);

compiler/rustc_hir_pretty/src/lib.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -960,12 +960,16 @@ impl<'a> State<'a> {
960960

961961
fn print_local(
962962
&mut self,
963+
super_: bool,
963964
init: Option<&hir::Expr<'_>>,
964965
els: Option<&hir::Block<'_>>,
965966
decl: impl Fn(&mut Self),
966967
) {
967968
self.space_if_not_bol();
968969
self.ibox(INDENT_UNIT);
970+
if super_ {
971+
self.word_nbsp("super");
972+
}
969973
self.word_nbsp("let");
970974

971975
self.ibox(INDENT_UNIT);
@@ -995,7 +999,9 @@ impl<'a> State<'a> {
995999
self.maybe_print_comment(st.span.lo());
9961000
match st.kind {
9971001
hir::StmtKind::Let(loc) => {
998-
self.print_local(loc.init, loc.els, |this| this.print_local_decl(loc));
1002+
self.print_local(loc.super_.is_some(), loc.init, loc.els, |this| {
1003+
this.print_local_decl(loc)
1004+
});
9991005
}
10001006
hir::StmtKind::Item(item) => self.ann.nested(self, Nested::Item(item)),
10011007
hir::StmtKind::Expr(expr) => {
@@ -1488,7 +1494,7 @@ impl<'a> State<'a> {
14881494

14891495
// Print `let _t = $init;`:
14901496
let temp = Ident::from_str("_t");
1491-
self.print_local(Some(init), None, |this| this.print_ident(temp));
1497+
self.print_local(false, Some(init), None, |this| this.print_ident(temp));
14921498
self.word(";");
14931499

14941500
// Print `_t`:

compiler/rustc_hir_typeck/src/gather_locals.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub(super) struct Declaration<'a> {
4343

4444
impl<'a> From<&'a hir::LetStmt<'a>> for Declaration<'a> {
4545
fn from(local: &'a hir::LetStmt<'a>) -> Self {
46-
let hir::LetStmt { hir_id, pat, ty, span, init, els, source: _ } = *local;
46+
let hir::LetStmt { hir_id, super_: _, pat, ty, span, init, els, source: _ } = *local;
4747
Declaration { hir_id, pat, ty, span, init, origin: DeclOrigin::LocalDecl { els } }
4848
}
4949
}

0 commit comments

Comments
 (0)