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

error message regression: "no resolution found" #38750

Closed
nrc opened this issue Jan 1, 2017 · 5 comments · Fixed by #38890
Closed

error message regression: "no resolution found" #38750

nrc opened this issue Jan 1, 2017 · 5 comments · Fixed by #38890
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@nrc
Copy link
Member

nrc commented Jan 1, 2017

"resolution"/"resolve" is compiler jargon and shouldn't be present in user-facing messages. Could be replaced by "declaration" or "definition", though I suspect we might be able to craft a better message than that.

cc @jseyfried @jonathandturner @GuillaumeGomez

@nrc nrc added A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Jan 1, 2017
@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 1, 2017

"resolution"/"resolve" is compiler jargon and shouldn't be present in user-facing messages.

This is news for me, the change was certainly intended.
Previously "unresolved" was used in patterns in struct expressions, #38154 unified error messages and now it's used everywhere.
There's also an independent error about "unresolved import"s.

Now "unresolved"/"resolution" is used twice in non-import paths - in the basic error message (unresolved <struct> <A::B>) and in the fallback label (no resolution found) when we can't say anything more useful.

Alternatives:
"undefined"
"undeclared"
"unknown"
"not in scope"
"can't find"

EDIT: for comparison, errors from clang and gcc:

// clang
main.cpp:2:14: error: use of undeclared identifier 'b'
    auto a = b;    
             ^

// gcc
main.cpp: In function 'int main()':
main.cpp:2:14: error: 'b' was not declared in this scope
     auto a = b;
              ^

@jseyfried
Copy link
Contributor

How about something like this?

use foo::bar; //~ ERROR no module `foo` in the crate root
//  ^^^^^^^^ `foo` undeclared
//  ^^^ undeclared (pending better span info)

mod baz {}
use baz::bar; //~ ERROR no item `bar` in `baz`
//       ^^^ undeclared

fn main() {
    foo; //~ ERROR no value `foo` in scope
//  ^^^ not in scope
}

@GuillaumeGomez
Copy link
Member

@jseyfried: I like your examples!
@nrc: it'd be more clear, indeed.

@nrc
Copy link
Member Author

nrc commented Jan 4, 2017

@petrochenkov I think I feel better with "resolved"/"unresolved" as verbs/adverbs than I do with "resolution" as a noun. I like your other suggestions (and @jseyfried's) better.

I would prefer more 'wordy' error messages (similar to the GCC/Clang) examples, rather than the shorter, note-like ones @jseyfried suggested, e.g., "bar is undeclared" rather than just "undeclared" or "'foo' was not declared in this scope" rather than "not in scope".

@jseyfried
Copy link
Contributor

@nrc

the shorter, note-like ones @jseyfried suggested

To be clear, I was only suggesting the shorter ones for the span_labels, e.g.

error: no value `foo` in scope
 --> <anon>:2:5
  |
2 |     foo;
  |     ^^^ not in scope

bors added a commit that referenced this issue Jan 13, 2017
resolve: Do not use "resolve"/"resolution" in error messages

Use less jargon-y wording instead.
`cannot find <struct> <S> in <this scope>` and `cannot find <struct> <S> in <module a::b>` are used for base messages (this also harmonizes nicely with "you can import it into scope" suggestions) and `not found in <this scope>` and `not found in <a::b>` are used for short labels in fall-back case.
I tweaked some other diagnostics to avoid using "resolve" (see, e.g., `librustc_resolve/macros.rs`), but haven't touched messages for imports.

Closes #38750
r? @nrc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants