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 suggest incorrect syntax #51670

Merged
merged 3 commits into from
Jun 22, 2018
Merged

Don't suggest incorrect syntax #51670

merged 3 commits into from
Jun 22, 2018

Conversation

estebank
Copy link
Contributor

Fix #51634.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2018

We probably get the same bad suggestion when creating the binding via a macro that takes the name of the local to be created via an ident fragment.

@estebank
Copy link
Contributor Author

Can you provide a repro case? I'll see if I have any way of differentiate them (other than checking the span context).

@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2018

somewhat xD I found other stuff:

    let x: _ = 42;
    x.pow(2);
help: you must specify a type for this binding, like `i32`
  |
8 |     let x: i32: _ = 42;
  |         ^^^^^^

and

macro_rules! foo {
    ($ident:ident) => { let $ident = 42; }
}

fn main() {
    foo!(bar);
    bar.pow(2);
}

suggests to modify a macro (which might be not from this crate)

help: you must specify a type for this binding, like `i32`
  |
2 |     ($ident:ident) => { let $ident: i32 = 42; }
  |                             ^^^^^^^^^^^ 

@estebank
Copy link
Contributor Author

@oli-obk as is, on external macros, the suggestion uses the compiled macro output:

error[E0689]: can't call method `pow` on ambiguous numeric type `{integer}`
  --> $DIR/method-on-ambiguous-numeric-type.rs:40:9
   |
LL |     bar.pow(2);
   |         ^^^
help: you must specify a type for this binding, like `i32`
   |
LL | ( $ ident : ident ) => { let $ ident: i32 = 42 ; }
   |                              ^^^^^^^^^^^^

@estebank
Copy link
Contributor Author

estebank commented Jun 21, 2018

Changed to account for both of the presented cases.

error[E0689]: can't call method `pow` on ambiguous numeric type `{integer}`
  --> $DIR/method-on-ambiguous-numeric-type.rs:40:9
   |
LL |     mac!(bar);
   |     ---------- you must specify a type for this binding, like `i32`
LL |     bar.pow(2);
   |         ^^^
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

match parent_node {
hir_map::NodeLocal(hir::Local {

match (is_real_filename, parent_node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You match on filename above toget this bool. Can you just match on it directly here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Jun 21, 2018

📌 Commit cc0ab82 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2018
@bors
Copy link
Contributor

bors commented Jun 22, 2018

⌛ Testing commit cc0ab82 with merge 8f02447...

bors added a commit that referenced this pull request Jun 22, 2018
Don't suggest incorrect syntax

Fix #51634.
@bors
Copy link
Contributor

bors commented Jun 22, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing 8f02447 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants