make missing_doc respect the visibility rules #10277
Conversation
|
This pass is becoming complex enough that it should no longer be using ad-hoc rules for determining whether an item is private or public. This pass was originally implemented before there was a comprehensive privacy analysis pass (explaining why it duplicates the logic of privacy). With that in mind, the code should get restructured to using a combination of the I think that the privacy pass may want to output a more usable form of information which is the set of all publicly reachable items (you currently must recompute that based off the two maps above), but that may be a separate issue as well. Regardless, seeing how this pass is getting more complicated, it's time to start using the real analysis pass of the compiler. Additionally, I believe that once the results of privacy are being used that the |
|
I think you're saying that the privacy pass should be changed so that After that is done, the |
|
The Otherwise yes, that's sounds like the way to go! |
|
This was a lot more complicated than I expected it to be, but I think it's ready for review now. |
| @@ -491,7 +507,7 @@ pub fn each_lint(sess: session::Session, | |||
| true | |||
| } | |||
|
|
|||
| fn check_while_true_expr(cx: &Context, e: &ast::Expr) { | |||
| fn check_while_true_expr<'a>(cx: &Context<'a>, e: &ast::Expr) { | |||
There was a problem hiding this comment.
If the Context is an argument, you shouldn't need an explicit lifetime parameter. It's inferred to be a "dummy parameter".
|
I'll take a closer look at this today or tomorrow, thanks for updating! |
|
The force-push removed lifetime parameters and made one other small tweak. |
|
Nice work on this! Also thanks for doing this, I really like where this is going. I've laid out comments here and there about things which I think need to get restructured, but feel free to ping me once updated! |
|
Revised as per feedback. |
|
Nice job! As one final thing, would you mind rebasing your commits into just one? |
Previously, the `exported_items` set created by the privacy pass was incomplete. Specifically, it did not include items that had been defined at a private path but then `pub use`d at a public path. This commit finds all crate exports during the privacy pass. Consequently, some code in the reachable pass and in rustdoc is no longer necessary. This commit then removes the separate `MissingDocLintVisitor` lint pass, opting to check missing_doc lint in the same pass as the other lint checkers using the visibility result computed by the privacy pass. Fixes rust-lang#9777.
|
Commits squashed into one. I figured it was easier to review logically distinct changes. |
|
It was, and thanks for that! After reviewing though, it seems like a good one-chunk change though to rebase into one (although it doesn't really matter that much) |
…r=alexcrichton Now the privacy pass returns enough information that other passes do not need to duplicate the visibility rules, and the missing_doc implementation is more consistent with other lint checks.
Now the privacy pass returns enough information that other passes do not need to duplicate the visibility rules, and the missing_doc implementation is more consistent with other lint checks.