Skip to content

Fix SourceScope for if let bindings. #97931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 20, 2022
Merged

Fix SourceScope for if let bindings. #97931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b48f3d8
Fix `SourceScope` for `if let` bindings.
xldenis Jun 9, 2022
53deccf
More minimal changes
xldenis Jun 9, 2022
cc75f0e
Revert "More minimal changes"
xldenis Jun 10, 2022
e39c92e
Actually fix
xldenis Jun 10, 2022
a5e27a1
Only create scopes for if let
xldenis Jun 10, 2022
a434e4c
Add debuginfo test for if let lexical scopes
wesleywiser Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/into.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
(if_then_scope, then_source_info),
LintLevel::Inherited,
|this| {
let source_info = if this.is_let(cond) {
let variable_scope = this.new_source_scope(
then_expr.span,
LintLevel::Inherited,
None,
);
this.source_scope = variable_scope;
SourceInfo { span: then_expr.span, scope: variable_scope }
} else {
this.source_info(then_expr.span)
};
let (then_block, else_block) =
this.in_if_then_scope(condition_scope, |this| {
let then_blk = unpack!(this.then_else_break(
block,
&this.thir[cond],
Some(condition_scope),
condition_scope,
then_expr.span,
source_info
));

this.expr_into_dest(destination, then_blk, then_expr)
});
then_block.and(else_block)
Expand All @@ -97,7 +109,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ExprKind::Let { expr, ref pat } => {
let scope = this.local_scope();
let (true_block, false_block) = this.in_if_then_scope(scope, |this| {
this.lower_let_expr(block, &this.thir[expr], pat, scope, expr_span)
this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span)
});

this.cfg.push_assign_constant(
Expand Down Expand Up @@ -575,4 +587,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

block_and
}

fn is_let(&self, expr: ExprId) -> bool {
match self.thir[expr].kind {
ExprKind::Let { .. } => true,
ExprKind::Scope { value, .. } => self.is_let(value),
_ => false,
}
}
}
39 changes: 29 additions & 10 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr: &Expr<'tcx>,
temp_scope_override: Option<region::Scope>,
break_scope: region::Scope,
variable_scope_span: Span,
variable_source_info: SourceInfo,
) -> BlockAnd<()> {
let this = self;
let expr_span = expr.span;
Expand All @@ -53,15 +53,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&this.thir[lhs],
temp_scope_override,
break_scope,
variable_scope_span,
variable_source_info,
));

let rhs_then_block = unpack!(this.then_else_break(
lhs_then_block,
&this.thir[rhs],
temp_scope_override,
break_scope,
variable_scope_span,
variable_source_info,
));

rhs_then_block.unit()
Expand All @@ -74,13 +74,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&this.thir[value],
temp_scope_override,
break_scope,
variable_scope_span,
variable_source_info,
)
})
}
ExprKind::Let { expr, ref pat } => {
this.lower_let_expr(block, &this.thir[expr], pat, break_scope, variable_scope_span)
}
ExprKind::Let { expr, ref pat } => this.lower_let_expr(
block,
&this.thir[expr],
pat,
break_scope,
Some(variable_source_info.scope),
variable_source_info.span,
),
_ => {
let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope());
let mutability = Mutability::Mut;
Expand Down Expand Up @@ -1773,6 +1778,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr: &Expr<'tcx>,
pat: &Pat<'tcx>,
else_target: region::Scope,
source_scope: Option<SourceScope>,
span: Span,
) -> BlockAnd<()> {
let expr_span = expr.span;
Expand All @@ -1798,7 +1804,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
self.break_for_else(otherwise_post_guard_block, else_target, self.source_info(expr_span));

self.declare_bindings(None, pat.span.to(span), pat, ArmHasGuard(false), opt_expr_place);
self.declare_bindings(
source_scope,
pat.span.to(span),
pat,
ArmHasGuard(false),
opt_expr_place,
);

let post_guard_block = self.bind_pattern(
self.source_info(pat.span),
guard_candidate,
Expand Down Expand Up @@ -1970,12 +1983,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Guard::If(e) => {
let e = &this.thir[e];
guard_span = e.span;
this.then_else_break(block, e, None, match_scope, arm_span)
this.then_else_break(
block,
e,
None,
match_scope,
this.source_info(arm_span),
)
}
Guard::IfLet(ref pat, scrutinee) => {
let s = &this.thir[scrutinee];
guard_span = s.span;
this.lower_let_expr(block, s, pat, match_scope, arm_span)
this.lower_let_expr(block, s, pat, match_scope, None, arm_span)
}
});

Expand Down
100 changes: 100 additions & 0 deletions src/test/debuginfo/lexical-scope-in-if-let.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// compile-flags:-g

// === GDB TESTS ==================================================================================

// gdb-command:run
// gdb-command:info locals
// gdb-check:a = 123

// gdb-command:continue
// gdb-command:info locals
// gdb-check:x = 42
// gdb-check:a = 123

// gdb-command:continue
// gdb-command:info locals
// gdb-check:y = true
// gdb-check:b = 456
// gdb-check:x = 42
// gdb-check:a = 123

// gdb-command:continue
// gdb-command:info locals
// gdb-check:z = 10
// gdb-check:c = 789
// gdb-check:y = true
// gdb-check:b = 456
// gdb-check:x = 42
// gdb-check:a = 123

// === LLDB TESTS =================================================================================

// lldb-command:run
// lldb-command:frame variable
// lldb-check:(int) a = 123

// lldb-command:continue
// lldb-command:frame variable
// lldb-check:(int) a = 123 (int) x = 42

// lldb-command:continue
// lldb-command:frame variable
// lldb-check:(int) a = 123 (int) x = 42 (int) b = 456 (bool) y = true

// lldb-command:continue
// lldb-command:frame variable
// lldb-check:(int) a = 123 (int) x = 42 (int) b = 456 (bool) y = true (int) c = 789 (int) z = 10

// === CDB TESTS ==================================================================================

// cdb-command: g
// cdb-command: dv
// cdb-check:[...]a = 0n123

// cdb-command: g
// cdb-command: dv
// cdb-check:[...]a = 0n123
// cdb-check:[...]x = 0n42

// cdb-command: g
// cdb-command: dv
// cdb-check:[...]y = true
// cdb-check:[...]b = 0n456
// cdb-check:[...]a = 0n123
// cdb-check:[...]x = 0n42

// cdb-command: g
// cdb-command: dv
// cdb-check:[...]z = 0n10
// cdb-check:[...]c = 0n789
// cdb-check:[...]y = true
// cdb-check:[...]b = 0n456
// cdb-check:[...]a = 0n123
// cdb-check:[...]x = 0n42

fn main() {
let a = id(123);

zzz(); // #break

if let Some(x) = id(Some(42)) {
zzz(); // #break

let b = id(456);

if let Ok(y) = id::<Result<bool, ()>>(Ok(true)) {
zzz(); // #break

let c = id(789);

if let (z, 42) = id((10, 42)) {
zzz(); // #break
}
}
}
}

#[inline(never)]
fn id<T>(value: T) -> T { value }

fn zzz() { }
22 changes: 12 additions & 10 deletions src/test/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,28 @@
scope 1 {
debug x => _1; // in scope 1 at $DIR/discriminant.rs:11:9: 11:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/discriminant.rs:11:9: 11:10
StorageLive(_2); // scope 0 at $DIR/discriminant.rs:11:13: 11:64
StorageLive(_3); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
Deinit(_3); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
((_3 as Some).0: bool) = const true; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
discriminant(_3) = 1; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
- _4 = discriminant(_3); // scope 0 at $DIR/discriminant.rs:11:21: 11:31
- switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ _4 = const 1_isize; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
StorageLive(_3); // scope 2 at $DIR/discriminant.rs:11:34: 11:44
Deinit(_3); // scope 2 at $DIR/discriminant.rs:11:34: 11:44
((_3 as Some).0: bool) = const true; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
discriminant(_3) = 1; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
- _4 = discriminant(_3); // scope 2 at $DIR/discriminant.rs:11:21: 11:31
- switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+ _4 = const 1_isize; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+ switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
}

bb1: {
switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
}

bb2: {
_2 = const 42_i32; // scope 0 at $DIR/discriminant.rs:11:47: 11:49
_2 = const 42_i32; // scope 2 at $DIR/discriminant.rs:11:47: 11:49
goto -> bb4; // scope 0 at $DIR/discriminant.rs:11:13: 11:64
}

Expand Down
22 changes: 12 additions & 10 deletions src/test/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,28 @@
scope 1 {
debug x => _1; // in scope 1 at $DIR/discriminant.rs:11:9: 11:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/discriminant.rs:11:9: 11:10
StorageLive(_2); // scope 0 at $DIR/discriminant.rs:11:13: 11:64
StorageLive(_3); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
Deinit(_3); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
((_3 as Some).0: bool) = const true; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
discriminant(_3) = 1; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
- _4 = discriminant(_3); // scope 0 at $DIR/discriminant.rs:11:21: 11:31
- switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ _4 = const 1_isize; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
StorageLive(_3); // scope 2 at $DIR/discriminant.rs:11:34: 11:44
Deinit(_3); // scope 2 at $DIR/discriminant.rs:11:34: 11:44
((_3 as Some).0: bool) = const true; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
discriminant(_3) = 1; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
- _4 = discriminant(_3); // scope 2 at $DIR/discriminant.rs:11:21: 11:31
- switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+ _4 = const 1_isize; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+ switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
}

bb1: {
switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
}

bb2: {
_2 = const 42_i32; // scope 0 at $DIR/discriminant.rs:11:47: 11:49
_2 = const 42_i32; // scope 2 at $DIR/discriminant.rs:11:47: 11:49
goto -> bb4; // scope 0 at $DIR/discriminant.rs:11:13: 11:64
}

Expand Down
18 changes: 10 additions & 8 deletions src/test/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,24 @@
let mut _2: isize; // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:20: 13:30
let mut _3: isize; // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
let mut _4: &E; // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:12:16: 12:17
scope 1 {
}

bb0: {
_3 = discriminant((*_1)); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
switchInt(move _3) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
_3 = discriminant((*_1)); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
switchInt(move _3) -> [1_isize: bb1, otherwise: bb3]; // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
}

bb1: {
StorageLive(_4); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
_4 = move (((*_1) as Some).0: &E); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
_2 = discriminant((*_4)); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
switchInt(move _2) -> [1_isize: bb2, otherwise: bb3]; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
StorageLive(_4); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
_4 = move (((*_1) as Some).0: &E); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
_2 = discriminant((*_4)); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
StorageDead(_4); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
switchInt(move _2) -> [1_isize: bb2, otherwise: bb3]; // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
}

bb2: {
_0 = const 1_u32; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:38: 13:39
_0 = const 1_u32; // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:38: 13:39
goto -> bb4; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:5: 13:52
}

Expand Down
Loading