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

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

Open
steffahn opened this issue Sep 27, 2022 · 1 comment
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@steffahn
Copy link
Member

steffahn commented Sep 27, 2022

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

@steffahn steffahn added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 27, 2022
@steffahn steffahn changed the title Printing of types expands type synonyms, but does not avoid naming collisions that can thus occur in HRTs Printing of types expands type synonyms, but does not avoid naming collisions of lifetimes that can thus occur in HRTs Sep 27, 2022
@steffahn steffahn changed the title Printing of types expands type synonyms, but does not avoid naming collisions of lifetimes that can thus occur in HRTs Printing of types expands type aliases, but does not avoid naming collisions of lifetimes that can thus occur in HRTs Sep 27, 2022
@b-naber
Copy link
Contributor

b-naber commented Sep 27, 2022

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants