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.

Sign up for GitHub

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

Jump to bottom

Fix rustdoc run failures by shutting down definitely some lints #51956

Merged
merged 3 commits into from
Jul 9, 2018
Merged

Fix rustdoc run failures by shutting down definitely some lints #51956

merged 3 commits into from
Jul 9, 2018

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2018
oli-obk
oli-obk reviewed Jun 30, 2018
View reviewed changes
src/librustdoc/core.rs Outdated
@@ -235,6 +235,14 @@ pub fn run_core(search_paths: SearchPaths,
let mut sess = session::build_session_(
sessopts, cpath, diagnostic_handler, codemap,
);

let shutdown_lints = [lint::builtin::UNUSED_IMPORTS,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait that's the wrong way around. I thought you were going to turn on the rustdoc lints even if lints are capped

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do, but I need to deactivate some lints even if they're enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just turn the cap lints back on as it was before

@oli-obk oli-obk added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 1, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jul 1, 2018

We should discuss this in the compiler meeting, because adding/changing lints in the compiler can now randomly break rustdoc for ppl with any kind of deny on, even if that deny does not cause any fallout during normal compilation

@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 3, 2018
Closed
oli-obk
oli-obk reviewed Jul 9, 2018
View reviewed changes
src/librustdoc/core.rs Outdated
if lint.name == warnings_lint_name ||
lint.name == intra_link_resolution_failure_name ||
lint.name == missing_docs {
None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a while to figure this out. So you are creating a list of all lints except for those you want to keep, and then you set all of them to allow.

Can you add a comment about this here?

Explanation about why I was confused:

I initially assumed we'd build a list of the lints we want to keep (so essentially vec![warnings_lint_name, intra_link_resolution_failure_name, missing_docs], set --cap-lints=allow and then make sure that cap-lints doesn't override our specific set of lints.

r=me with comment

@GuillaumeGomez
Copy link
Member Author

@bors: r=oli-obk

@bors
Copy link
Contributor

bors commented Jul 9, 2018

📌 Commit 66beb4e has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2018
@bors
Copy link
Contributor

bors commented Jul 9, 2018

⌛ Testing commit 66beb4e with merge bdd185c...

bors added a commit that referenced this pull request Jul 9, 2018
@bors
Auto merge of #51956 - GuillaumeGomez:shutdown-doc-lints, r=oli-obk
bdd185c 
Fix rustdoc run failures by shutting down definitely some lints

Fixes #51661.

cc @oli-obk @arielb1 @eddyb
@bors
Copy link
Contributor

bors commented Jul 9, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing bdd185c to master...

@bors bors merged commit 66beb4e into rust-lang:master Jul 9, 2018
@GuillaumeGomez GuillaumeGomez deleted the shutdown-doc-lints branch July 10, 2018 08:34
@pietroalbini pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 10, 2018
@pietroalbini
Copy link
Member

Nominating for beta backport since this fixes a regression

@pnkfelix pnkfelix mentioned this pull request Jul 12, 2018
Closed
@pietroalbini pietroalbini added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 14, 2018
@pietroalbini
Copy link
Member

Marked as beta-accepted (#52313).

@pietroalbini pietroalbini mentioned this pull request Jul 14, 2018
Merged
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 14, 2018
bors added a commit that referenced this pull request Jul 14, 2018
@bors
Auto merge of #52366 - pietroalbini:beta-backports, r=pietroalbini
67e5a18 
[beta] Rollup backports

Merged and approved:

* #51956: Fix rustdoc run failures by shutting down definitely some lints
* #52232: use the adjusted type for cat_pattern in tuple patterns

Closes #52311
Closes #52313

r? @ghost
@ollie27 ollie27 mentioned this pull request Jul 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@michaelwoerister michaelwoerister

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @rust-highfive @oli-obk @bors @pietroalbini @michaelwoerister