On privacy error caused by private reexport, use spans to show the use chain#67951
On privacy error caused by private reexport, use spans to show the use chain#67951estebank wants to merge 1 commit intorust-lang:masterfrom
use chain#67951Conversation
|
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
@petrochenkov would you have some advice on how to get a better suited source for a usable path string in resolve?
There was a problem hiding this comment.
There are zero guarantees that a direct path won't have private segments, especially given that module structures with facades are pretty popular.
So trying to guess and use full paths here means inviting false positives.
I'd rather keep the status quo and say only the name.
(In this case the changes in src/librustc/hir/map/definitions.rs can be removed as well, those strings are not user-facing otherwise and are used only for debugging.)
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
@petrochenkov would you have some advice on how to properly check the visibility of a given ADT here? I'm guessing I would have to ferry some scope information in PrivacyError.
There was a problem hiding this comment.
This issue doesn't apply to the simpler version of this PR (#67951 (comment)) which I'd like to see implemented first.
I'm not sure we'll need anything beyond that.
varkor
left a comment
There was a problem hiding this comment.
I really like these more informative errors! They could just do with a little tweaking for readability.
45e2df4 to
883a86b
Compare
|
I'm not sure that reporting the full reexports chains is useful. So, I'd replace this: with this This would be a pretty minimal change to the code existing on master. |
The concern I have is that by only stating "you're importing something private, and this is the private re-export" and nothing else we don't give users a chance of changing their |
|
I'd still want to see the diff between the basic version and what this PR does, so I made #68153. |
|
Blocked on #68153 and #67951 (comment). |
resolve: Point at the private item definitions in privacy errors A basic version of rust-lang#67951. r? @estebank
resolve: Point at the private item definitions in privacy errors A basic version of rust-lang#67951. r? @estebank
resolve: Point at the private item definitions in privacy errors A basic version of rust-lang#67951. r? @estebank
This comment has been minimized.
This comment has been minimized.
|
@estebank this is unblocked now |
…se` chain Use full path for direct `use` of ADT instead of private re-export Point at enum defs and modules on private re-exports Use span notes to denote order Account for `use` of private `extern crate foo;`
65c8532 to
b3a554d
Compare
|
@Dylan-DPC thanks for the ping. @petrochenkov rebased and squashed on top of latest master, the diff is much smaller now. |
| --> $DIR/issue-55884-2.rs:6:13 | ||
| | | ||
| LL | pub use options::*; | ||
| | ^^^^^^^^^^ |
There was a problem hiding this comment.
I think this clarification in particular will be incredibly useful when trying to understand these errors.
|
I will get to this next weekend. |
|
I'll close this in favor of #69811 which does roughly the same thing. |
resolve: Print import chains on privacy errors A part of rust-lang#67951 that doesn't require hacks. r? @estebank
resolve: Print import chains on privacy errors A part of rust-lang#67951 that doesn't require hacks. r? @estebank
resolve: Print import chains on privacy errors A part of rust-lang#67951 that doesn't require hacks. r? @estebank
resolve: Print import chains on privacy errors A part of rust-lang#67951 that doesn't require hacks. r? @estebank
Fix #63942, fix #57619. Partially address #42909.