Skip to content

Commit 4bce3fd

Browse files
committed
Auto merge of rust-lang#139112 - m-ou-se:super-let, r=<try>
Implement `super let` Tracking issue: rust-lang#139076 This implements `super let` as proposed in rust-lang#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 rust-lang#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 f174fd7 + 0bec927 commit 4bce3fd

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
@@ -1169,6 +1169,7 @@ pub enum MacStmtStyle {
11691169
#[derive(Clone, Encodable, Decodable, Debug)]
11701170
pub struct Local {
11711171
pub id: NodeId,
1172+
pub super_: Option<Span>,
11721173
pub pat: P<Pat>,
11731174
pub ty: Option<P<Ty>>,
11741175
pub kind: LocalKind,
@@ -3926,7 +3927,7 @@ mod size_asserts {
39263927
static_assert_size!(Item, 144);
39273928
static_assert_size!(ItemKind, 80);
39283929
static_assert_size!(LitKind, 24);
3929-
static_assert_size!(Local, 80);
3930+
static_assert_size!(Local, 96);
39303931
static_assert_size!(MetaItemLit, 40);
39313932
static_assert_size!(Param, 40);
39323933
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
@@ -2223,6 +2223,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
22232223
self.attrs.insert(hir_id.local_id, a);
22242224
}
22252225
let local = hir::LetStmt {
2226+
super_: None,
22262227
hir_id,
22272228
init,
22282229
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
@@ -1817,6 +1817,8 @@ pub enum StmtKind<'hir> {
18171817
/// Represents a `let` statement (i.e., `let <pat>:<ty> = <init>;`).
18181818
#[derive(Debug, Clone, Copy, HashStable_Generic)]
18191819
pub struct LetStmt<'hir> {
1820+
/// Span of `super` in `super let`.
1821+
pub super_: Option<Span>,
18201822
pub pat: &'hir Pat<'hir>,
18211823
/// Type annotation, if any (otherwise the type will be inferred).
18221824
pub ty: Option<&'hir Ty<'hir>>,
@@ -4850,7 +4852,7 @@ mod size_asserts {
48504852
static_assert_size!(ImplItemKind<'_>, 40);
48514853
static_assert_size!(Item<'_>, 88);
48524854
static_assert_size!(ItemKind<'_>, 64);
4853-
static_assert_size!(LetStmt<'_>, 64);
4855+
static_assert_size!(LetStmt<'_>, 72);
48544856
static_assert_size!(Param<'_>, 32);
48554857
static_assert_size!(Pat<'_>, 72);
48564858
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>(
@@ -481,14 +495,19 @@ fn resolve_expr<'tcx>(
481495
visitor.cx = prev_cx;
482496
}
483497

498+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
499+
enum LetKind {
500+
Regular,
501+
Super,
502+
}
503+
484504
fn resolve_local<'tcx>(
485505
visitor: &mut ScopeResolutionVisitor<'tcx>,
486506
pat: Option<&'tcx hir::Pat<'tcx>>,
487507
init: Option<&'tcx hir::Expr<'tcx>>,
508+
let_kind: LetKind,
488509
) {
489-
debug!("resolve_local(pat={:?}, init={:?})", pat, init);
490-
491-
let blk_scope = visitor.cx.var_parent;
510+
debug!("resolve_local(pat={:?}, init={:?}, let_kind={:?})", pat, init, let_kind);
492511

493512
// As an exception to the normal rules governing temporary
494513
// lifetimes, initializers in a let have a temporary lifetime
@@ -546,14 +565,50 @@ fn resolve_local<'tcx>(
546565
// A, but the inner rvalues `a()` and `b()` have an extended lifetime
547566
// due to rule C.
548567

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

552604
if let Some(pat) = pat {
553605
if is_binding_pat(pat) {
554606
visitor.scope_tree.record_rvalue_candidate(
555607
expr.hir_id,
556-
RvalueCandidate { target: expr.hir_id.local_id, lifetime: blk_scope },
608+
RvalueCandidate {
609+
target: expr.hir_id.local_id,
610+
lifetime: visitor.cx.var_parent,
611+
},
557612
);
558613
}
559614
}
@@ -565,6 +620,7 @@ fn resolve_local<'tcx>(
565620
if let Some(expr) = init {
566621
visitor.visit_expr(expr);
567622
}
623+
568624
if let Some(pat) = pat {
569625
visitor.visit_pat(pat);
570626
}
@@ -642,6 +698,7 @@ fn resolve_local<'tcx>(
642698
/// | [ ..., E&, ... ]
643699
/// | ( ..., E&, ... )
644700
/// | {...; E&}
701+
/// | { super let ... = E&; ... }
645702
/// | if _ { ...; E& } else { ...; E& }
646703
/// | match _ { ..., _ => E&, ... }
647704
/// | box E&
@@ -678,6 +735,13 @@ fn resolve_local<'tcx>(
678735
if let Some(subexpr) = block.expr {
679736
record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
680737
}
738+
for stmt in block.stmts {
739+
if let hir::StmtKind::Let(local) = stmt.kind
740+
&& let Some(_) = local.super_
741+
{
742+
visitor.extended_super_lets.insert(local.pat.hir_id.local_id, blk_id);
743+
}
744+
}
681745
}
682746
hir::ExprKind::If(_, then_block, else_block) => {
683747
record_rvalue_scope_if_borrow_expr(visitor, then_block, blk_id);
@@ -803,7 +867,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
803867
local_id: body.value.hir_id.local_id,
804868
data: ScopeData::Destruction,
805869
});
806-
resolve_local(this, None, Some(body.value));
870+
resolve_local(this, None, Some(body.value), LetKind::Regular);
807871
}
808872
})
809873
}
@@ -821,7 +885,11 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
821885
resolve_expr(self, ex, false);
822886
}
823887
fn visit_local(&mut self, l: &'tcx LetStmt<'tcx>) {
824-
resolve_local(self, Some(l.pat), l.init)
888+
let let_kind = match l.super_ {
889+
Some(_) => LetKind::Super,
890+
None => LetKind::Regular,
891+
};
892+
resolve_local(self, Some(l.pat), l.init, let_kind);
825893
}
826894
fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
827895
let body = self.tcx.hir_body(c.body);
@@ -850,6 +918,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
850918
cx: Context { parent: None, var_parent: None },
851919
pessimistic_yield: false,
852920
fixup_scopes: vec![],
921+
extended_super_lets: Default::default(),
853922
};
854923

855924
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)