typeck: if a private field exists, also check for a public method #33342
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
|
r? @Aatch (rust_highfive has picked a reviewer for you, use r? to override) |
steveklabnik
added a commit
to steveklabnik/rust
that referenced
this pull request
May 2, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
|
I believe there was some discussion in internals.rust-lang.org about making these kinds of messages |
steveklabnik
added a commit
to steveklabnik/rust
that referenced
this pull request
May 5, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
|
@bors: r- another commit got added here, I don't know why bors still thought it was reviewed |
|
It also appears to have failed the rollup http://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt/builds/4104/steps/compile/logs/stdio |
|
@steveklabnik I didn't add another commit, I rebased and fixed up the existing after seeing that the rollup failed. |
| // Also check if an accessible method exists, which is often what is meant. | ||
| if method::exists(fcx, field.span, field.node, expr_t, expr.id, false) { | ||
| err.note(&format!("a method called `{}` also exists, \ | ||
| maybe a `()` to call it is missing?", |
There was a problem hiding this comment.
nit: a method named {} or just `a method `{}
Also, I would prefer perhaps you wish to call it as in this error message (instead of maybe a () to call it is missing?).
|
r=me with the nit addressed |
|
r? @jseyfried |
|
Thanks! |
|
📌 Commit 84034d4 has been approved by |
steveklabnik
added a commit
to steveklabnik/rust
that referenced
this pull request
May 10, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
steveklabnik
added a commit
to steveklabnik/rust
that referenced
this pull request
May 10, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
|
☔ The latest upstream changes (presumably #33425) made this pull request unmergeable. Please resolve the merge conflicts. |
eddyb
added a commit
to eddyb/rust
that referenced
this pull request
May 12, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
eddyb
added a commit
to eddyb/rust
that referenced
this pull request
May 13, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
sanxiyn
added a commit
to sanxiyn/rust
that referenced
this pull request
May 14, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
bors
added a commit
that referenced
this pull request
May 14, 2016
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
May 15, 2016
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
bors
added a commit
that referenced
this pull request
May 15, 2016
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.
For example,
Vec::lenis both a field and a method, and usually encounteringvec.lenjust means that the parens were forgotten.Fixes: #26472
NOTE: I added the parameter
allow_privatetomethod::existssince I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it tofalseas well, but I wanted to preserve compatibility for this case.