-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Improve unknown external crate error #81046
Conversation
The job Click to see the possible cause of the failure (guessed by this bot)
|
28d9ee4
to
d51df05
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
let parent = if parent == kw::PathRoot { | ||
"crate root".to_owned() | ||
} else { | ||
format!("`{}`", parent) | ||
}; |
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.
Rather than special casing this here, does it make sense to change in impl Display for Symbol
instead? Then you don't have to worry about missing it somewhere (and everyone else using this benefits as well).
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.
This is certainly a possibility. I didn't do this because I was unsure of the ramifications of renaming the symbol's string representation (particularly around json serialization/deserialization). The change I made seems to be the central place for diagnostics around path resolution (i.e., there are not UI tests that still reference {{root}}
).
format!("could not find `{}` in `{}`", ident, path[i - 1].ident); | ||
let parent = path[i - 1].ident.name; | ||
let parent = if parent == kw::PathRoot { | ||
"crate root".to_owned() |
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.
Should it be root crate
instead?
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.
Hmmm, the path root is normally referred to as the crate root as you can see here. root crate
makes it sound like this refers to a crate itself but it does not, it's the root of the namespace hierarchy.
d51df05
to
d829e40
Compare
@bors r+ rollup |
📌 Commit 38b7742 has been approved by |
…ebank Improve unknown external crate error This improves error messages when unknown items in the crate root are encountered. Fixes rust-lang#63799 r? `@estebank`
…ebank Improve unknown external crate error This improves error messages when unknown items in the crate root are encountered. Fixes rust-lang#63799 r? ``@estebank``
Rollup of 11 pull requests Successful merges: - rust-lang#79655 (Add Vec visualization to understand capacity) - rust-lang#80172 (Use consistent punctuation for 'Prelude contents' docs) - rust-lang#80429 (Add regression test for mutual recursion in obligation forest) - rust-lang#80601 (Improve grammar in documentation of format strings) - rust-lang#81046 (Improve unknown external crate error) - rust-lang#81178 (Visit only terminators when removing landing pads) - rust-lang#81179 (Fix broken links with `--document-private-items` in the standard library) - rust-lang#81184 (Remove unnecessary `after_run` function) - rust-lang#81185 (Fix ICE in mir when evaluating SizeOf on unsized type) - rust-lang#81187 (Fix typo in counters.rs) - rust-lang#81219 (Document security implications of std::env::temp_dir) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This improves error messages when unknown items in the crate root are encountered.
Fixes #63799
r? @estebank