Skip to content

Commit

Permalink
Auto merge of rust-lang#83870 - jackh726:binder-refactor-fix, r=nikom…
Browse files Browse the repository at this point in the history
…atsakis

Don't concatenate binders across types

Partially addresses rust-lang#83737

There's actually two issues that I uncovered in rust-lang#83737. The first is that we are concatenating bound vars across types, i.e. in
```
F: Fn(&()) -> &mut (dyn Future<Output = ()> + Unpin)
```
the bound vars on `Future` get set as `for<anon>` since those are the binders on `Fn(&()`. This is obviously wrong, since we should only concatenate directly nested trait refs. This is solved here by introducing a new `TraitRefBoundary` scope, that we put around the "syntactical" trait refs and basically don't allow concatenation across.

Now, this alone *shouldn't* be a super terrible problem. At least not until you consider the other issue, which is a much more elusive and harder to design a "perfect" fix. A repro can be seen in:
```
use core::future::Future;

async fn handle<F>(slf: &F)
where
    F: Fn(&()) -> &mut (dyn for<'a> Future<Output = ()> + Unpin),
{
    (slf)(&()).await;
}
```
Notice the `for<'a>` around `Future`. Here, `'a` is unused, so the `for<'a>` Binder gets changed to a `for<>` Binder in the generator witness, but the "local decl" still has it. This has heavy intersections with region anonymization and erasing. Luckily, it's not *super* common to find this unique set of circumstances. It only became apparently because of the first issue mentioned here. However, this *is* still a problem, so I'm leaving rust-lang#83737 open.

r? `@nikomatsakis`
  • Loading branch information
bors committed Apr 9, 2021
2 parents 619d19c + c1dc0b7 commit cd56e25
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 86 deletions.
109 changes: 87 additions & 22 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1804,29 +1804,94 @@ impl<F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
define_scoped_cx!(self);

let mut region_index = self.region_index;
let new_value = self.tcx.replace_late_bound_regions(value.clone(), |br| {
let _ = start_or_continue(&mut self, "for<", ", ");
let kind = match br.kind {
ty::BrNamed(_, name) => {
let _ = write!(self, "{}", name);
br.kind
}
ty::BrAnon(_) | ty::BrEnv => {
let name = loop {
let name = name_by_region_index(region_index);
region_index += 1;
if !self.used_region_names.contains(&name) {
break name;
}
};
let _ = write!(self, "{}", name);
ty::BrNamed(DefId::local(CRATE_DEF_INDEX), name)
// If we want to print verbosly, then print *all* binders, even if they
// aren't named. Eventually, we might just want this as the default, but
// this is not *quite* right and changes the ordering of some output
// anyways.
let new_value = if self.tcx().sess.verbose() {
// anon index + 1 (BrEnv takes 0) -> name
let mut region_map: BTreeMap<u32, Symbol> = BTreeMap::default();
let bound_vars = value.bound_vars();
for var in bound_vars {
match var {
ty::BoundVariableKind::Region(ty::BrNamed(_, name)) => {
let _ = start_or_continue(&mut self, "for<", ", ");
let _ = write!(self, "{}", name);
}
ty::BoundVariableKind::Region(ty::BrAnon(i)) => {
let _ = start_or_continue(&mut self, "for<", ", ");
let name = loop {
let name = name_by_region_index(region_index);
region_index += 1;
if !self.used_region_names.contains(&name) {
break name;
}
};
let _ = write!(self, "{}", name);
region_map.insert(i + 1, name);
}
ty::BoundVariableKind::Region(ty::BrEnv) => {
let _ = start_or_continue(&mut self, "for<", ", ");
let name = loop {
let name = name_by_region_index(region_index);
region_index += 1;
if !self.used_region_names.contains(&name) {
break name;
}
};
let _ = write!(self, "{}", name);
region_map.insert(0, name);
}
_ => continue,
}
};
self.tcx
.mk_region(ty::ReLateBound(ty::INNERMOST, ty::BoundRegion { var: br.var, kind }))
});
start_or_continue(&mut self, "", "> ")?;
}
start_or_continue(&mut self, "", "> ")?;

self.tcx.replace_late_bound_regions(value.clone(), |br| {
let kind = match br.kind {
ty::BrNamed(_, _) => br.kind,
ty::BrAnon(i) => {
let name = region_map[&(i + 1)];
ty::BrNamed(DefId::local(CRATE_DEF_INDEX), name)
}
ty::BrEnv => {
let name = region_map[&0];
ty::BrNamed(DefId::local(CRATE_DEF_INDEX), name)
}
};
self.tcx.mk_region(ty::ReLateBound(
ty::INNERMOST,
ty::BoundRegion { var: br.var, kind },
))
})
} else {
let new_value = self.tcx.replace_late_bound_regions(value.clone(), |br| {
let _ = start_or_continue(&mut self, "for<", ", ");
let kind = match br.kind {
ty::BrNamed(_, name) => {
let _ = write!(self, "{}", name);
br.kind
}
ty::BrAnon(_) | ty::BrEnv => {
let name = loop {
let name = name_by_region_index(region_index);
region_index += 1;
if !self.used_region_names.contains(&name) {
break name;
}
};
let _ = write!(self, "{}", name);
ty::BrNamed(DefId::local(CRATE_DEF_INDEX), name)
}
};
self.tcx.mk_region(ty::ReLateBound(
ty::INNERMOST,
ty::BoundRegion { var: br.var, kind },
))
});
start_or_continue(&mut self, "", "> ")?;
new_value
};

self.binder_depth += 1;
self.region_index = region_index;
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_mir/src/transform/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,9 +751,10 @@ fn sanitize_witness<'tcx>(
span_bug!(
body.span,
"Broken MIR: generator contains type {} in MIR, \
but typeck only knows about {}",
decl.ty,
witness,
but typeck only knows about {} and {:?}",
decl_ty,
allowed,
allowed_upvars
);
}
}
Expand Down
Loading

0 comments on commit cd56e25

Please sign in to comment.