-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add note when a function is called from a module like it's an object #30356
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
Conversation
74b373f
to
66f5576
Compare
expr.span, | ||
PathSearch) { | ||
Success(_) => { | ||
help_msg = format!("A module called `{module}` is \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
To reference an item from the
{module}
module, use{module}::{next_path_component}
.
instead? It is shorter and more-to-the-point. Also this obviously should become a span_help
.
E.g. for your case it would then look like this:
To reference an item from the
io
module, useio::stdin
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
Tests? |
} | ||
|
||
resolve_error(self, | ||
expr.span, | ||
ResolutionError::UnresolvedName(&*path_name, &*msg)); | ||
if !help_msg.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the help msg should be passed down to resolve_error? I think the rest of the file does help messages this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I infer @Manishearth is referring to how a case like OnlyIrrefutablePatternsAllowedHere
is handled; see linked code; following such a strategy would entail either changing the UnresolvedName
variant, or adding a new variant that carries the necessary info... but nonetheless, it is probably a good suggestion, just to keep all the error reporting in one place...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind, it's the same for me. I'll just provide an Option
as extra parameter and we're good to go.
66f5576
to
bf3e37d
Compare
@nagisa: Since it doesn't add a new error code, is it really needed ? I can add one, but if there is already one for this, it'll be useless... |
There should always be tests. Error codes are orthogonal. |
To be clearer, notes should have tests too. If there is a test, update it to mention the note, otherwise create your own. |
PS: To add a "note" tested comment: //~NOTE |
@@ -3391,6 +3391,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { | |||
|
|||
self.record_candidate_traits_for_expr_if_necessary(expr); | |||
|
|||
expr.toto(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test of mine, wasn't intended to get committed. Just ignore it please.
@pnkfelix found a way to handle 2 of 4 cases with the following code. of this example: mod a { mod b { fn f() -> i32 { 3 } } }
fn g1() -> i32 {
a.b.f()
}
fn g2() -> i32 {
a::b.f()
}
fn g3() -> i32 {
a::b()
}
fn g4() -> i32 {
a::b
}
fn main() {
println!("g1: {} g2: {} g3: {}", g1(), g2(), g3());
} I'll cherry-pick his work and try to handle the last case. |
closing in favor of PR #30413 (where @GuillaumeGomez still gets attribution) |
Add note when item accessed from module via `m.i` rather than `m::i`. (I tried to make this somewhat future-proofed, in that the `UnresolvedNameContext` could be expanded in the future with other cases besides paths that are known to be modules.) This supersedes PR #30356 ; since I'm responsible for a bunch of new code here, someone else should review it. :)
The following code:
Which prints out:
Some beginners had issues with this error because they didn't understood it, so I added an explanation which helps them to understand it better.
r? @pnkfelix
cc @Manishearth
cc @nagisa