Skip to content

mir: don't nest the initialization scope in the remainder scope. #33235

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

Closed
wants to merge 1 commit into from
Closed

mir: don't nest the initialization scope in the remainder scope. #33235

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 13 additions & 9 deletions src/librustc_mir/build/block.rs
View file
Original file line number Diff line number Diff line change
@@ -53,16 +53,20 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => {
let remainder_scope_id = this.push_scope(remainder_scope, block);
this.declare_bindings(remainder_scope_id, &pattern);
let_extent_stack.push(remainder_scope);
unpack!(block = this.in_scope(init_scope, block, move |this, _| {
// FIXME #30046 ^~~~
if let Some(init) = initializer {
this.expr_into_pattern(block, remainder_scope_id, pattern, init)
} else {
this.declare_bindings(remainder_scope_id, &pattern);
block.unit()
}
}));

let parent = this.scope_datas[remainder_scope_id].parent_scope;
if let Some(init) = initializer {
unpack!(block = this.in_scope(init_scope, block, move |this, id| {
// FIXME #30046 ^~~~

// Don't nest the initializer in the remainder scope.
this.scope_datas[id].parent_scope = parent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutating this imperatively feels really hacky. Maybe add a parameter to in_scope? I forget how often we call this.

I'm still not sure if this change is OK-- but I think it's fine to refine as we go.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real problem is that we need to create the remainder scope, place variables and schedule drops in it, without entering it.
Do you think it's feasible to disconnect "create a scope" from "push a ScopeId"?
That might also help with the scope bloat, if nested expression scopes can reuse the same ScopeId, I would think.


this.expr_into_pattern(block, pattern, init)
}));
}
}
}
}
26 changes: 4 additions & 22 deletions src/librustc_mir/build/matches/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -116,45 +116,27 @@ impl<'a,'tcx> Builder<'a,'tcx> {

pub fn expr_into_pattern(&mut self,
mut block: BasicBlock,
var_scope_id: ScopeId, // lifetime of vars
irrefutable_pat: Pattern<'tcx>,
initializer: ExprRef<'tcx>)
-> BlockAnd<()> {
// optimize the case of `let x = ...`
match *irrefutable_pat.kind {
PatternKind::Binding { mutability,
name,
mode: BindingMode::ByValue,
var,
ty,
subpattern: None } => {
let index = self.declare_binding(var_scope_id,
mutability,
name,
var,
ty,
irrefutable_pat.span);
let lvalue = Lvalue::Var(index);
PatternKind::Binding { mode: BindingMode::ByValue,
var, subpattern: None, .. } => {
let lvalue = Lvalue::Var(self.var_indices[&var]);
return self.into(&lvalue, block, initializer);
}
_ => {}
}
let lvalue = unpack!(block = self.as_lvalue(block, initializer));
self.lvalue_into_pattern(block,
var_scope_id,
irrefutable_pat,
&lvalue)
self.lvalue_into_pattern(block, irrefutable_pat, &lvalue)
}

pub fn lvalue_into_pattern(&mut self,
mut block: BasicBlock,
var_scope_id: ScopeId,
irrefutable_pat: Pattern<'tcx>,
initializer: &Lvalue<'tcx>)
-> BlockAnd<()> {
// first, creating the bindings
self.declare_bindings(var_scope_id, &irrefutable_pat);

// create a dummy candidate
let mut candidate = Candidate {
span: irrefutable_pat.span,
17 changes: 10 additions & 7 deletions src/librustc_mir/build/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -283,19 +283,22 @@ impl<'a,'tcx> Builder<'a,'tcx> {
.enumerate()
.map(|(index, (ty, pattern))| {
let lvalue = Lvalue::Arg(index as u32);
if let Some(pattern) = pattern {
let pattern = self.hir.irrefutable_pat(pattern);
unpack!(block = self.lvalue_into_pattern(block,
argument_scope_id,
pattern,
&lvalue));
}

// Make sure we drop (parts of) the argument even when not matched on.
let argument_extent = self.scope_auxiliary[argument_scope_id].extent;
self.schedule_drop(pattern.as_ref().map_or(ast_block.span, |pat| pat.span),
argument_extent, &lvalue, ty);

if let Some(pattern) = pattern {
let pattern = self.hir.irrefutable_pat(pattern);
self.declare_bindings(argument_scope_id, &pattern);

// Don't have the argument scope around when matching the pattern.
let arg_scope = self.scopes.pop().unwrap();
unpack!(block = self.lvalue_into_pattern(block, pattern, &lvalue));
self.scopes.push(arg_scope);
}

let mut name = keywords::Invalid.name();
if let Some(pat) = pattern {
if let hir::PatKind::Ident(_, ref ident, _) = pat.node {
13 changes: 1 addition & 12 deletions src/test/debuginfo/function-prologue-stepping-no-stack-check.rs
View file
Original file line number Diff line number Diff line change
@@ -247,11 +247,10 @@
// lldb-command:continue

#![allow(dead_code, unused_assignments, unused_variables)]
#![feature(omit_gdb_pretty_printer_section, rustc_attrs)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn immediate_args(a: isize, b: bool, c: f64) {
println!("");
}
@@ -268,51 +267,43 @@ struct BigStruct {
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn non_immediate_args(a: BigStruct, b: BigStruct) {
println!("");
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn binding(a: i64, b: u64, c: f64) {
let x = 0;
println!("");
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn assignment(mut a: u64, b: u64, c: f64) {
a = b;
println!("");
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn function_call(x: u64, y: u64, z: f64) {
println!("Hi!")
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn identifier(x: u64, y: u64, z: f64) -> u64 {
x
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn return_expr(x: u64, y: u64, z: f64) -> u64 {
return x;
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn arithmetic_expr(x: u64, y: u64, z: f64) -> u64 {
x + y
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn if_expr(x: u64, y: u64, z: f64) -> u64 {
if x + y < 1000 {
x
@@ -322,7 +313,6 @@ fn if_expr(x: u64, y: u64, z: f64) -> u64 {
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn while_expr(mut x: u64, y: u64, z: u64) -> u64 {
while x + y < 1000 {
x += z
@@ -331,7 +321,6 @@ fn while_expr(mut x: u64, y: u64, z: u64) -> u64 {
}

#[no_stack_check]
#[rustc_no_mir] // FIXME(#32949) MIR debuginfo shadows arguments with uninit vars.
fn loop_expr(mut x: u64, y: u64, z: u64) -> u64 {
loop {
x += z;