Cleanup rustdoc lint listing

[GuillaumeGomez]

Fixes #78786.

Explanations: we need a compiler instance in order to be able to get a LintStore which allows us to get access to the "rustdoc" lint group. It has the advantage that we don't require to keep this list up-to-date by manually anymore. Maybe it might be worth a perf check? Didn't see an impact when running locally but maybe there might be one?

r? @jyn514

[jyn514]

This is a hack, but I think it's better than the existing mess. I don't think you need a full perf run, but if you can add tcx.sess.time("init lints") to see how long get_rustdoc_lints takes to execute that would be nice (I expect not very long).

[jyn514]

This should not be mixed in with the cleanup; it's the same sort of change as #77364, which I'm still not convinced is the right change.

[GuillaumeGomez]

It's only emitted by rustdoc, so the context is a bit different there. This whole PR wouldn't be possible without this change because otherwise I'll need to still handle this lint differently than the others. However, because it was a bit out of the scope of this PR, I have put this change in a commit on its own (it's the first commit, you can check ;) ).

[jyn514]

Suggested change

	// In addition to those specific lints, we also need to allow those given through
	// command line, otherwise they'll get ignored and we don't want that.
	// Rustdoc silences all lints by default, only warning on lints in the `rustdoc` group.
	// These lints aren't in the lint group, but we want to warn on them anyway.

[jyn514]

Also, if I understand correctly, rustdoc will no longer warn even if you add -W non_upper_case_globals or something like that, is that correct? I'm ok with that change personally, but it needs an FCP IMO.

[GuillaumeGomez]

It currently doesn't. I tried with this code:

pub static foo: u32 = 0;

fn main() {}

Then I ran:

$ rustdoc foo.rs -W non_upper_case_globals
$ rustc foo.rs -W non_upper_case_globals
warning: static variable `foo` should have an upper case name
 --> foo.rs:1:12
  |
1 | pub static foo: u32 = 0;
  |            ^^^ help: convert the identifier to upper case (notice the capitalization): `FOO`
  |
  = note: requested on the command line with `-W non-upper-case-globals`

warning: 1 warning emitted

So no changes there as far as I can see.

[GuillaumeGomez]

Just for info, here are the versions:

$ rustdoc --version
rustdoc 1.51.0-nightly (2987785df 2020-12-28)
$ rustc --version
rustc 1.51.0-nightly (2987785df 2020-12-28)

[jyn514]

This is a change in behavior (the same one from #73314). I think it's a good change, but it shouldn't be mixed in with a cleanup.

[GuillaumeGomez]

No it's not. Look just below: I set lint_opts after using the get_rustdoc_lints function. So it's the same but applied in two steps. I need this first one to do the least amount of things as possible.

[GuillaumeGomez]

To be exact, it's done here:

if !display_warnings {
    sessopts.lint_opts = lint_opts;
}

[jyn514]

If you remove this, does it still warn on those lints?

[GuillaumeGomez]

I didn't want to update it in this PR to avoid mixing too many things together. We can take a look when this is merged if you want.

[GuillaumeGomez]

This is a hack, but I think it's better than the existing mess. I don't think you need a full perf run, but if you can add tcx.sess.time("init lints") to see how long get_rustdoc_lints takes to execute that would be nice (I expect not very long).

Only problem with that is that I don't have a tcx nor a Session at this point. 😞

[GuillaumeGomez]

So in the end, I just applied the comment update you suggested. 😆

[GuillaumeGomez]

Since there is now #80527, this PR is not necessary anymore. Closing it!