Remove feed_local_def_id by oli-obk · Pull Request #121089 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation37 Commits9 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on #121084
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
This comment was marked as resolved.
This comment was marked as resolved.
📌 Commit a640db5 has been approved by cjgillot
It is now in the queue for this repository.
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
I don't understand what we get from this, to be honest.
What are the additional guarantees from keeping Feeds in node_id_to_def_id tables?
What are the additional guarantees from keeping
Feeds innode_id_to_def_idtables?
that you don't feed from another query but the one that created the DefId. Feed cannot be transferred across query boundaries.
feed_local_def_id is a massive footgun and if anone uses it from anywhere but the resolver it will cause undebuggable incremental ICEs. The reason the resolver is fine is that it is (like the analysis query, an outermost (not called from another query) eval_always query.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
What are the additional guarantees from keeping
Feeds innode_id_to_def_idtables?that you don't feed from another query but the one that created the
DefId.Feedcannot be transferred across query boundaries.
feed_local_def_idis a massive footgun and if anone uses it from anywhere but the resolver it will cause undebuggable incremental ICEs. The reason the resolver is fine is that it is (like theanalysisquery, an outermost (not called from another query)eval_alwaysquery.
I probably makes sense to put this explanation as a comment on struct Feed.
Feedcannot be transferred across query boundaries.
I'm not sure if it's a hard guarantee, or something that we need to keep in mind when writing new code.
For example, what happens if we put Feed into ResolverOutputs?
Then it will be usable from other queries through tcx.resolutions(()).
This comment has been minimized.
This comment was marked as resolved.
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
I'm not sure if it's a hard guarantee
I'll add a negative impl for the relevant traits
This comment has been minimized.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
This comment has been minimized.
r=me after updating the failing test.
@rustbot author
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
📌 Commit ef00fae has been approved by petrochenkov
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…enkov
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…enkov
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#113518 (bootstrap/libtest: print test name eagerly on failure even with
verbose-tests=false/--quiet) - rust-lang#119888 (Stabilize the
#[diagnostic]namespace and#[diagnostic::on_unimplemented]attribute) - rust-lang#121089 (Remove
feed_local_def_id) - rust-lang#121926 (
f16andf128step 3: compiler support & feature gate) - rust-lang#121959 (Removing absolute path in proc-macro)
- rust-lang#122015 (Add better explanation for
rustc_index::IndexVec) - rust-lang#122027 (Uplift some feeding out of
associated_type_for_impl_trait_in_impland into queries) - rust-lang#122038 (Fix linting paths with qself in
unused_qualifications)
r? @ghost
@rustbot modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request
…enkov
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
bors added a commit to rust-lang-ci/rust that referenced this pull request
bors added a commit to rust-lang-ci/rust that referenced this pull request
…llaumeGomez
Rollup of 10 pull requests
Successful merges:
- rust-lang#119888 (Stabilize the
#[diagnostic]namespace and#[diagnostic::on_unimplemented]attribute) - rust-lang#121089 (Remove
feed_local_def_id) - rust-lang#122004 (AST validation: Improve handling of inherent impls nested within functions and anon consts)
- rust-lang#122087 (Add missing background color for top-level rust documentation page and increase contrast by setting text color to black)
- rust-lang#122136 (Include all library files in artifact summary on CI)
- rust-lang#122137 (Don't pass a break scope to
Builder::break_for_else) - rust-lang#122138 (Record mtime in bootstrap's LLVM linker script)
- rust-lang#122141 (sync (try_)instantiate_mir_and_normalize_erasing_regions implementation)
- rust-lang#122142 (cleanup rustc_infer)
- rust-lang#122147 (Make
std::os::unix::ucredmodule private)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#121089 - oli-obk:create_def_feed, r=petrochenkov
Remove feed_local_def_id
best reviewed commit by commit
Basically I returned TyCtxtFeed from create_def and then preserved that in the local caches
based on rust-lang#121084
r? @petrochenkov
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.