Skip to content
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.

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

Don't repeat lifetime names from outer binder in print #102514

Merged
merged 4 commits into from
Oct 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
69 changes: 57 additions & 12 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2055,7 +2055,14 @@ struct RegionFolder<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
current_index: ty::DebruijnIndex,
region_map: BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
name: &'a mut (dyn FnMut(ty::BoundRegion) -> ty::Region<'tcx> + 'a),
name: &'a mut (
dyn FnMut(
Option<ty::DebruijnIndex>,
ty::DebruijnIndex,
b-naber marked this conversation as resolved.
Show resolved Hide resolved
ty::BoundRegion,
) -> ty::Region<'tcx>
+ 'a
),
}

impl<'a, 'tcx> ty::TypeFolder<'tcx> for RegionFolder<'a, 'tcx> {
Expand Down Expand Up @@ -2086,7 +2093,9 @@ impl<'a, 'tcx> ty::TypeFolder<'tcx> for RegionFolder<'a, 'tcx> {
fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
let name = &mut self.name;
let region = match *r {
ty::ReLateBound(_, br) => *self.region_map.entry(br).or_insert_with(|| name(br)),
ty::ReLateBound(db, br) => {
Copy link
Contributor

@lcnr lcnr Oct 6, 2022

Choose a reason for hiding this comment

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

that's weird, why is this not

Suggested change
ty::ReLateBound(db, br) => {
ty::ReLateBound(db, br) if db == self.current_index => {

we shouldn't replace late-bound regions of inner binders.

Copy link
Contributor

Choose a reason for hiding this comment

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

(ah, that's whatyou're currently doing inside of name. i think it's better to do this here instead)

*self.region_map.entry(br).or_insert_with(|| name(Some(db), self.current_index, br))
}
ty::RePlaceholder(ty::PlaceholderRegion { name: kind, .. }) => {
// If this is an anonymous placeholder, don't rename. Otherwise, in some
// async fns, we get a `for<'r> Send` bound
Expand All @@ -2095,7 +2104,10 @@ impl<'a, 'tcx> ty::TypeFolder<'tcx> for RegionFolder<'a, 'tcx> {
_ => {
// Index doesn't matter, since this is just for naming and these never get bound
let br = ty::BoundRegion { var: ty::BoundVar::from_u32(0), kind };
*self.region_map.entry(br).or_insert_with(|| name(br))
*self
.region_map
.entry(br)
.or_insert_with(|| name(None, self.current_index, br))
Copy link
Contributor

Choose a reason for hiding this comment

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

is that the issue why we're not printing the binder in the src/test/ui/where-clauses/higher-ranked-fn-type.rs test?

We have both a late-bound region with name 'b and index 0 and a placeholder with name 'b and index 0.

I am not too confident in this code and ideally we stop printing placeholders as if they were bound regions.

@jackh726 do you have an idea how this should work or any expectations for how difficult it is to remove the placeholder hack all-together

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like the reason this isn't printed is that we skip the binder in the error reporting code?!

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, we shouldn't do that. Either map_bound or replace with placeholders. Skipping binders is incorrect for errors as well i think

}
}
}
Expand Down Expand Up @@ -2234,24 +2246,57 @@ impl<'tcx> FmtPrinter<'_, 'tcx> {
})
} else {
let tcx = self.tcx;
let mut name = |br: ty::BoundRegion| {
start_or_continue(&mut self, "for<", ", ");
let kind = match br.kind {
let mut name = |db: Option<ty::DebruijnIndex>,
b-naber marked this conversation as resolved.
Show resolved Hide resolved
binder_level: ty::DebruijnIndex,
br: ty::BoundRegion| {
let (name, kind) = match br.kind {
ty::BrAnon(_) | ty::BrEnv => {
let name = next_name(&self);
do_continue(&mut self, name);
ty::BrNamed(CRATE_DEF_ID.to_def_id(), name)

if let Some(db) = db {
if db > binder_level {
let kind = ty::BrNamed(CRATE_DEF_ID.to_def_id(), name);
return tcx.mk_region(ty::ReLateBound(
ty::INNERMOST,
ty::BoundRegion { var: br.var, kind },
));
}
}

(name, ty::BrNamed(CRATE_DEF_ID.to_def_id(), name))
}
ty::BrNamed(def_id, kw::UnderscoreLifetime) => {
let name = next_name(&self);
do_continue(&mut self, name);
ty::BrNamed(def_id, name)

if let Some(db) = db {
if db > binder_level {
let kind = ty::BrNamed(def_id, name);
return tcx.mk_region(ty::ReLateBound(
ty::INNERMOST,
ty::BoundRegion { var: br.var, kind },
));
}
}

(name, ty::BrNamed(def_id, name))
}
ty::BrNamed(_, name) => {
do_continue(&mut self, name);
br.kind
if let Some(db) = db {
if db > binder_level {
let kind = br.kind;
return tcx.mk_region(ty::ReLateBound(
ty::INNERMOST,
ty::BoundRegion { var: br.var, kind },
));
}
}

(name, br.kind)
}
};

start_or_continue(&mut self, "for<", ", ");
do_continue(&mut self, name);
tcx.mk_region(ty::ReLateBound(ty::INNERMOST, ty::BoundRegion { var: br.var, kind }))
};
let mut folder = RegionFolder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ LL | async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
| ___________________________________________________________________^
LL | | }
| |_^
= note: required because it captures the following types: `ResumeTy`, `impl for<'a, 'b, 'c> Future<Output = ()>`, `()`
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
note: required because it's used within this `async` block
--> $DIR/issue-70935-complex-spans.rs:16:16
|
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0119]: conflicting implementations of trait `Trait` for type `for<'r> fn(for<'r> fn(&'r ()))`
error[E0119]: conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))`
--> $DIR/coherence-fn-covariant-bound-vs-static.rs:17:1
|
LL | impl Trait for for<'r> fn(fn(&'r ())) {}
| ------------------------------------- first implementation here
LL | impl<'a> Trait for fn(fn(&'a ())) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'r> fn(for<'r> fn(&'r ()))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'r> fn(fn(&'r ()))`
|
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetimes/re-empty-in-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: higher-ranked lifetime error
LL | foo(&10);
| ^^^^^^^^
|
= note: could not prove `for<'b, 'a> &'b (): 'a`
= note: could not prove `for<'b> &'b (): 'a`

error: aborting due to previous error

6 changes: 6 additions & 0 deletions src/test/ui/regions/issue-102392.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn g(f: for<'a> fn(fn(&str, &'a str))) -> bool {
f
//~^ ERROR mismatched types
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/regions/issue-102392.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0308]: mismatched types
--> $DIR/issue-102392.rs:2:5
|
LL | fn g(f: for<'a> fn(fn(&str, &'a str))) -> bool {
| ---- expected `bool` because of return type
LL | f
| ^ expected `bool`, found fn pointer
|
= note: expected type `bool`
found fn pointer `for<'a> fn(for<'b> fn(&'b str, &'a str))`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
4 changes: 2 additions & 2 deletions src/test/ui/where-clauses/higher-ranked-fn-type.quiet.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `for<'b> for<'b> fn(&'b ()): Foo` is not satisfied
error[E0277]: the trait bound `for<'b> fn(&'b ()): Foo` is not satisfied
--> $DIR/higher-ranked-fn-type.rs:20:5
|
LL | called()
| ^^^^^^ the trait `for<'b> Foo` is not implemented for `for<'b> fn(&'b ())`
| ^^^^^^ the trait `for<'b> Foo` is not implemented for `fn(&'b ())`
b-naber marked this conversation as resolved.
Show resolved Hide resolved
|
note: required by a bound in `called`
--> $DIR/higher-ranked-fn-type.rs:12:25
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/where-clauses/higher-ranked-fn-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ where
(for<'a> fn(&'a ())): Foo,
{
called()
//[quiet]~^ ERROR the trait bound `for<'b> for<'b> fn(&'b ()): Foo` is not satisfied
//[quiet]~^ ERROR the trait bound `for<'b> fn(&'b ()): Foo` is not satisfied
//[verbose]~^^ ERROR the trait bound `for<'b> fn(&ReLateBound(
}

Expand Down