Skip to content

Improve error message for one named, one anonymous lifetime parameters - underline Type #43851

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

Jump to bottom
Closed
gaurikholkar-zz wants to merge 2 commits into from
Closed

Improve error message for one named, one anonymous lifetime parameters - underline Type #43851

gaurikholkar-zz wants to merge 2 commits into from

Conversation

@gaurikholkar-zz
Copy link

@gaurikholkar-zz gaurikholkar-zz commented Aug 13, 2017 •
edited
Loading

This is a fix to #43433

Please ignore the previous commits. Only review
changing error message format for E0621

The other review is being done in PR #43700
Sample error message

error[E0621]: explicit lifetime required in the type of `x`
  --> $DIR/ex1-return-one-existing-name-if-else-2.rs:12:16
   |
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |               ---- consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                ^ lifetime `'a` required

error: aborting due to previous error

r? @nikomatsakis
cc @estebank @arielb1

@gaurikholkar-zz gaurikholkar-zz mentioned this pull request Aug 13, 2017
Closed
@gaurikholkar-zz gaurikholkar-zz mentioned this pull request Aug 14, 2017
Merged
@gaurikholkar-zz gaurikholkar-zz force-pushed the E0611 branch 2 times, most recently from bf5557e to 64488a4 Compare August 14, 2017 06:51
@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 14, 2017
@gaurikholkar-zz gaurikholkar-zz force-pushed the E0611 branch 2 times, most recently from e3d4f5e to a361a2c Compare August 16, 2017 14:51
nikomatsakis
nikomatsakis requested changes Aug 17, 2017
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

I'm a bit nervous about this change unless we also change the message somewhat -- see my comment below. What do you think?

src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr
|
11 | fn foo<'a>((x, y): (&'a i32, &i32)) -> &'a i32 {
| ------ consider changing type to `(&'a i32, &'a i32)`
| ---- consider changing type to `(&'a i32, &'a i32)`
Copy link
Contributor

@nikomatsakis nikomatsakis Aug 17, 2017

Choose a reason for hiding this comment

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

Hmm, so here we are underlining only a piece of the type, but the replacement that we are suggesting is the entire type. This seems a bit confusing to me.

@gaurikholkar-zz
Copy link
Author

gaurikholkar-zz commented Aug 21, 2017

Currently closing the issue. As per my discussions with @nikomatsakis, we have decided to go ahead with E0621 for now. Opening up an issue for the same and updating it with the observations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikomatsakis nikomatsakis nikomatsakis requested changes

Assignees

@nikomatsakis nikomatsakis

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gaurikholkar-zz @nikomatsakis @carols10cents @gaurikholkar