Closes #52413: Provide structured suggestion instead of label #52450
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
estebank
reviewed
Jul 17, 2018
| arg.pat.span, | ||
| format!("consider changing {} to `{}`", span_label_var, new_ty), | ||
| &format!("consider changing {} to `{}`", span_label_var, new_ty), | ||
| "".to_string() |
There was a problem hiding this comment.
When applying the suggestion, the code will be incorrect. The fourth argument should the new_ty, as that's what the code needs to be after applying the change, and the span needs to be changed to point at the argument type, instead of the argument name so the suggestion gets applied in the right place.
There was a problem hiding this comment.
@estebank make sense! I will fix this by today or tomorrow.
There was a problem hiding this comment.
@estebank I am stuck on this, I am not able to figure out how to find hir_id or node_id of arguments type at https://github.com/rust-lang/rust/blob/9f9ac89d11a2afaeaa029738b38142124684c89b/src/librustc/infer/error_reporting/nice_region_error/util.rs#L83
I had planning of introducing arg_ty_span in AnonymousArgInfo which later we can use at https://github.com/rust-lang/rust/blob/9f9ac89d11a2afaeaa029738b38142124684c89b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs#L115
Could you give me pointer or two on how to approach this. I think this is bit tough to find span on util.rs because in some cases table may not be fully populated.
PS: apology for delayed response :)
There was a problem hiding this comment.
@estebank I think I have found the way to find span of argument type. doing experiements now.
There was a problem hiding this comment.
@estebank: done :)
stokhos
added
S-waiting-on-author
PramodBisht
force-pushed
the
issue/52413
branch
from
efd6373
to
1940108
Compare
PramodBisht
commented
Aug 5, 2018
| | consider changing the type of `y` to `&'a u32` | ||
| | ---- ^ lifetime `'a` required | ||
| | | | ||
| | help: consider changing the type of `y` to : `&'a u32` |
There was a problem hiding this comment.
@estebank should we change this message? because with : it looks bit weird. Any suggestion?
There was a problem hiding this comment.
Lets reword it slightly, so it reads help: add explicit lifetime 'a to the type of 'y': '&'a u32', which is slightly better. The colon is always there for suggestions. If the message is long, or there are multiple possible suggestions, or the suggestion spans multiple lines the emitter will look like this:
error[E0621]: explicit lifetime required in the type of `y`
--> $DIR/mismatched.rs:14:42
|
LL | fn foo(x: &'a u32, y: &u32) -> &'a u32 { y } //~ ERROR explicit lifetime required
| ^ lifetime `'a` required
help: add explicit lifetime `'a` to the type of `y`
|
XX | fn foo(x: &'a u32, y: &'a u32) -> &'a u32 { y }
| ^^^^^^^
Because of this, wording needs to be able to work for both types of presentation.
There was a problem hiding this comment.
done!
r? @estebank
PramodBisht
added
S-waiting-on-review
|
Love the change! r=me after rewording the suggestion message. |
|
@bors r+ |
|
📌 Commit 49b0a1e has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 49b0a1e with merge ca33855a0553719b119a883c10d97fc82104e333... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
|
@bors retry travis-ci/travis-ci#9696 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provide structured suggestion instead of label
r? @estebank