Refactor: Move the mutable parts out of LintStore. Fix #42007. #42052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* rust-lang#42007 happens because the Session LintStore is emptied when linting. * The Session LintStore is emptied because the checker (Early/LateContext) wants ownership. * The checker wants ownership because it wants to mutate the pass objects and lint levels. The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the LintStore into a separate structure `LintSession`. The "check crates" methods can operate on `&mut LintSession` instead of `&mut LintStore`. This is a minor BREAKING CHANGE for lint writers since the `LintContext` trait is changed: the `mut_lints` and `level_stack` methods are removed. But no one outside of `librustc/lint/context.rs` is using these functions, so it should be safe.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
aidanhs
added
the
S-waiting-on-review
|
ping @nikomatsakis! pinging you on irc too :) |
nikomatsakis
approved these changes
May 22, 2017
|
|
||
| impl<'a, PassObject: LintPassObject> LintSession<'a, PassObject> { | ||
| /// Creates a new `LintSession`, by moving out the `LintStore`'s initial | ||
| /// lint levels and pass objects. These can be restored using the `restore` |
There was a problem hiding this comment.
So, I guess we're doing this "take and replace" dance because it's more robust? Otherwise, I'd say let's just clone() the LintLevels.
|
This seems like a net win. The lint setup still feels a bit baroque to me, and I'd like to refactor it more broadly (as I mentioned on IRC) to be better integrated with incremental compilation, but I think we can land this in the meantime. |
|
@bors r+ |
|
📌 Commit b384b18 has been approved by |
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this pull request
May 23, 2017
…t-id, r=nikomatsakis Refactor: Move the mutable parts out of LintStore. Fix rust-lang#42007. * rust-lang#42007 happens because the `Session` `LintStore` is emptied when linting. * The `Session` `LintStore` is emptied because the checker (`Early`/`LateContext`) wants ownership. * The checker wants ownership because it wants to mutate the pass objects and lint levels. The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the `LintStore` into a separate structure `LintSession`. The "check crates" methods can operate on `&mut LintSession` instead of `&mut LintStore`. This is a minor *breaking change* for lint writers since the `LintContext` trait is changed: the `mut_lints` and `level_stack` methods are removed. But no one outside of `librustc/lint/context.rs` is using these functions, so it should be safe.
arielb1
added
S-waiting-on-bors
|
⌛ Testing commit b384b18 with merge 8fe49ee... |
|
💔 Test failed - status-appveyor |
|
@bors retry
|
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
May 24, 2017
…t-id, r=nikomatsakis Refactor: Move the mutable parts out of LintStore. Fix rust-lang#42007. * rust-lang#42007 happens because the `Session` `LintStore` is emptied when linting. * The `Session` `LintStore` is emptied because the checker (`Early`/`LateContext`) wants ownership. * The checker wants ownership because it wants to mutate the pass objects and lint levels. The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the `LintStore` into a separate structure `LintSession`. The "check crates" methods can operate on `&mut LintSession` instead of `&mut LintStore`. This is a minor *breaking change* for lint writers since the `LintContext` trait is changed: the `mut_lints` and `level_stack` methods are removed. But no one outside of `librustc/lint/context.rs` is using these functions, so it should be safe.
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
May 24, 2017
…t-id, r=nikomatsakis Refactor: Move the mutable parts out of LintStore. Fix rust-lang#42007. * rust-lang#42007 happens because the `Session` `LintStore` is emptied when linting. * The `Session` `LintStore` is emptied because the checker (`Early`/`LateContext`) wants ownership. * The checker wants ownership because it wants to mutate the pass objects and lint levels. The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the `LintStore` into a separate structure `LintSession`. The "check crates" methods can operate on `&mut LintSession` instead of `&mut LintStore`. This is a minor *breaking change* for lint writers since the `LintContext` trait is changed: the `mut_lints` and `level_stack` methods are removed. But no one outside of `librustc/lint/context.rs` is using these functions, so it should be safe.
bors
added a commit
that referenced
this pull request
May 24, 2017
bors
added a commit
that referenced
this pull request
May 25, 2017
…omatsakis Refactor: Move the mutable parts out of LintStore. Fix #42007. * #42007 happens because the `Session` `LintStore` is emptied when linting. * The `Session` `LintStore` is emptied because the checker (`Early`/`LateContext`) wants ownership. * The checker wants ownership because it wants to mutate the pass objects and lint levels. The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the `LintStore` into a separate structure `LintSession`. The "check crates" methods can operate on `&mut LintSession` instead of `&mut LintStore`. This is a minor *breaking change* for lint writers since the `LintContext` trait is changed: the `mut_lints` and `level_stack` methods are removed. But no one outside of `librustc/lint/context.rs` is using these functions, so it should be safe.
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SessionLintStoreis emptied when linting.SessionLintStoreis emptied because the checker (Early/LateContext) wants ownership.The ownership of the whole store is not essential, only the lint levels and pass objects need to be owned. Therefore, these parts are extracted out of the
LintStoreinto a separate structureLintSession. The "check crates" methods can operate on&mut LintSessioninstead of&mut LintStore.This is a minor breaking change for lint writers since the
LintContexttrait is changed: themut_lintsandlevel_stackmethods are removed. But no one outside oflibrustc/lint/context.rsis using these functions, so it should be safe.