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.

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

Remove Sync requirement from lint pass objects #101156

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Aug 29, 2022

This is blocking the clippy sync (#101140). One of the lint passes contains a Cell in order to make lifetimes work. It could be worked around, but this is the easier change to make if there are no objections.

Rational for removing the requirement

  • All lint pass methods take &mut self arguments.
  • Many passes depend on running is visitor order.
  • Lint passes are created on demand so they're only ever stored in a local.
  • Send is enough to lint different passes in parallel.

LintStore remains Sync with this. The constructor functions it contains still maintain their Sync requirement.

r? rust-lang/compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 29, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2022
@compiler-errors
Copy link
Member

Does the parallel compiler build with this change?

@Jarcho
Copy link
Contributor Author

Jarcho commented Aug 29, 2022

It builds locally. CI is also building the parallel compiler is it not?

@compiler-errors
Copy link
Member

I don't think CI builds the parallel compiler, like it doesn't even build more than just the x86_64-unknown-linux-gnu compiler in stage2. But I did check that enabling the parallel compiler and building this branch works, so this is fine then.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2022

📌 Commit 74f2d58 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Aug 29, 2022
@Jarcho
Copy link
Contributor Author

Jarcho commented Aug 29, 2022

The failure in #101140 only happens when building the parallel compiler, so I had assumed that was what CI was doing.

@compiler-errors
Copy link
Member

Guess it does then 👍

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2022
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#100898 (Do not report too many expr field candidates)
 - rust-lang#101056 (Add the syntax of references to their documentation summary.)
 - rust-lang#101106 (Rustdoc-Json: Retain Stripped Modules when they are imported, not when they have items)
 - rust-lang#101131 (CTFE: exposing pointers and calling extern fn is just impossible)
 - rust-lang#101141 (Simplify `get_trait_ref` fn used for `virtual_function_elimination`)
 - rust-lang#101146 (Various changes to logging of borrowck-related code)
 - rust-lang#101156 (Remove `Sync` requirement from lint pass objects)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bf42ba4 into rust-lang:master Aug 30, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants