rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.58k stars 12.74k forks source link

Printing of types expands type aliases, but does not avoid naming collisions of lifetimes that can thus occur in HRTs #102346

Open steffahn opened 2 years ago

steffahn commented 2 years ago

Follow-up to #101280, which has been fixed in #101996, but that issue only explicitly addressed the case when the name was chosen by the compiler.

We can also get cases where all names are user-provided. E.g.

use std::cell::Cell;
type Ty1 = for<'r> fn(Cell<(&'r i32, &'r i32)>);
type Ty2<'a> = for<'r> fn(Cell<(&'a i32, &'r i32)>);
fn f<'r>(f: Ty1) -> Ty2<'r> {
    f
}
error[E0308]: mismatched types
 --> src/main.rs:9:5
  |
8 | fn f<'r>(f: Ty1) -> Ty2<'r> {
  |                     ------- expected `for<'r> fn(Cell<(&'r i32, &'r i32)>)` because of return type
9 |     f
  |     ^ one type is more general than the other
  |
  = note: expected fn pointer `for<'r> fn(Cell<(&'r i32, &'r i32)>)`
             found fn pointer `for<'r> fn(Cell<(&'r i32, &'r i32)>)`

I would expect the compiler to rename the bound variable in this case, e.g.

8 | fn f<'r>(f: Ty1) -> Ty2<'r> {
  |                     ------- expected `for<'r1> fn(Cell<(&'r i32, &'r1 i32)>)` because of return type
  = note: expected fn pointer `for<'r1> fn(Cell<(&'r i32, &'r1 i32)>)`
             found fn pointer `for<'r1> fn(Cell<(&'r1 i32, &'r1 i32)>)`

It might make sense to conservatively do such renaming in all cases where a lifetime of the same name is in scope, in the spirit of why using such a HRT directly (without type alias) is a compilation error. This is why in the suggested fix for the concrete error message above, the type for<'r1> fn(Cell<(&'r1 i32, &'r1 i32)>) has had its bound lifetime renamed, too.

It also seems reasonable to base the new name on the old one in some manner, as user-chosen names can be conveying useful information. Adding trailing numbers seems reasonable, but other approaches might be good, too. Of course, the idea when adding a number is that if a name 'r1 was already in scope, too, then 'r2 or 'r3, etc… would be chosen. I’m not sure if there’s precedent of choosing new names based on existing ones in the compiler. If the approach of adding a trailing number is chosen, then there’s the case to consider where the name already has a trailing number, in which case, either an additional number could be added, perhaps with underscore separation, or the existing number could be incremented. E.g. if the user named the lifetime 'r42, a new name could be 'r42_1 or 'r_43 or 'r1 (provided it’s still available).


cc @b-naber

b-naber commented 2 years ago

@rustbot claim