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

Switch to EarlyBinder for type_of query #107753

Merged
merged 4 commits into from
Feb 17, 2023
Merged

Conversation

kylematsuda
Copy link
Contributor

Part of the work to finish #105779 and implement rust-lang/types-team#78.

Several queries X have a bound_X variant that wraps the output in EarlyBinder. This adds EarlyBinder to the return type of the type_of query and removes bound_type_of.

r? @lcnr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 7, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 7, 2023

Some changes occurred in need_type_info.rs

cc @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Copy link
Contributor Author

@kylematsuda kylematsuda left a comment

Choose a reason for hiding this comment

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

Here are the places where I thought there were potential substs but wasn't sure whether they should be used

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved
compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved
compiler/rustc_borrowck/src/universal_regions.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_analysis/src/variance/constraints.rs Outdated Show resolved Hide resolved
@@ -906,8 +906,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// different from the received one
// So we avoid suggestion method with Box<Self>
// for instance
self.tcx.at(span).type_of(*def_id) != rcvr_ty
&& self.tcx.at(span).type_of(*def_id) != rcvr_ty
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is sort of unrelated to adding EarlyBinder, but I was confused by this... what's going on here?

Copy link
Member

Choose a reason for hiding this comment

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

yeah this is really odd lol I have no idea why its checking the same condition twice...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, so I did a little digging on this and tracked it down to #104422 (which renamed actual to rcvr_ty, whereas originally both names were in scope). I might try to think of a test case where the original check is needed

compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/print/pretty.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Feb 8, 2023

☔ The latest upstream changes (presumably #107768) made this pull request unmergeable. Please resolve the merge conflicts.

@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 8, 2023

r? @BoxyUwU

@rustbot rustbot assigned BoxyUwU and unassigned lcnr Feb 8, 2023
@lcnr
Copy link
Contributor

lcnr commented Feb 8, 2023

handing this off to @BoxyUwU as I don't have a lot of time atm ^^

I realized is that the reason I didn't see your comments in your previous PR was that they are for the commit which uses bound_X everywhere and I tend to only review the the final state of the PR. For future PRs it is probably better to only comment on the final changes as dealing with comments on specific diffs isn't great.

Thanks for your work ❤️

@bors
Copy link
Contributor

bors commented Feb 13, 2023

☔ The latest upstream changes (presumably #107924) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_hir_typeck/src/demand.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/method/confirm.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/method/probe.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/consts.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@kylematsuda
Copy link
Contributor Author

Hi @BoxyUwU, thanks for reviewing! I implemented all of those changes in the 2nd to last commit; the last commit is just a small change that came up when I rebased 😃

@bors
Copy link
Contributor

bors commented Feb 15, 2023

☔ The latest upstream changes (presumably #108079) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 15, 2023

☔ The latest upstream changes (presumably #108006) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 16, 2023

☔ The latest upstream changes (presumably #108096) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 16, 2023

☔ The latest upstream changes (presumably #108020) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9556b56): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% [-2.4%, -2.4%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% [-3.9%, -1.2%] 9
All ❌✅ (primary) - - 0

Jarcho pushed a commit to Jarcho/rust that referenced this pull request Feb 26, 2023
Switch to `EarlyBinder` for `type_of` query

Part of the work to finish rust-lang#105779 and implement rust-lang/types-team#78.

Several queries `X` have a `bound_X` variant that wraps the output in `EarlyBinder`. This adds `EarlyBinder` to the return type of the `type_of` query and removes `bound_type_of`.

r? `@lcnr`
qinheping added a commit to qinheping/kani that referenced this pull request Mar 10, 2023
Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
Unify validity checks into a single query rust-lang/rust#108364
s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
Switch to EarlyBinder for type_of query rust-lang/rust#107753
Rename interner funcs rust-lang/rust#108250
Optimize mk_region rust-lang/rust#108020
Clarify iterator interners rust-lang/rust#108112
qinheping added a commit to qinheping/kani that referenced this pull request Mar 10, 2023
- Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
- Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
- Unify validity checks into a single query rust-lang/rust#108364
- s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
- Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Rename interner funcs rust-lang/rust#108250
- Optimize mk_region rust-lang/rust#108020
- Clarify iterator interners rust-lang/rust#108112
tautschnig pushed a commit to qinheping/kani that referenced this pull request Mar 17, 2023
- Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
- Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
- Unify validity checks into a single query rust-lang/rust#108364
- s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
- Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Rename interner funcs rust-lang/rust#108250
- Optimize mk_region rust-lang/rust#108020
- Clarify iterator interners rust-lang/rust#108112
tautschnig pushed a commit to qinheping/kani that referenced this pull request Mar 17, 2023
- Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
- Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
- Unify validity checks into a single query rust-lang/rust#108364
- s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
- Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Rename interner funcs rust-lang/rust#108250
- Optimize mk_region rust-lang/rust#108020
- Clarify iterator interners rust-lang/rust#108112
tautschnig pushed a commit to qinheping/kani that referenced this pull request Mar 17, 2023
- Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
- Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
- Unify validity checks into a single query rust-lang/rust#108364
- s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
- Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Rename interner funcs rust-lang/rust#108250
- Optimize mk_region rust-lang/rust#108020
- Clarify iterator interners rust-lang/rust#108112
tautschnig pushed a commit to qinheping/kani that referenced this pull request Mar 17, 2023
- Remove some superfluous type parameters from layout.rs rust-lang/rust#107163
- Introduce -Zterminal-urls to use OSC8 for error codes rust-lang/rust#107838
- Unify validity checks into a single query rust-lang/rust#108364
- s/eval_usize/eval_target_usize/ for clarity rust-lang/rust#108029
- Use target instead of machine for mir interpreter integer handling rust-lang/rust#108047
- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Rename interner funcs rust-lang/rust#108250
- Optimize mk_region rust-lang/rust#108020
- Clarify iterator interners rust-lang/rust#108112
tautschnig added a commit to tautschnig/kani that referenced this pull request Apr 17, 2023
Upstream PRs that require local changes:

- Switch to EarlyBinder for type_of query rust-lang/rust#107753
- Factor query arena allocation out from query caches rust-lang/rust#107833

Co-authored-by: Qinheping Hu <[email protected]>
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request May 8, 2023
… r=BoxyUwU

Make `(try_)subst_and_normalize_erasing_regions` take `EarlyBinder`

Changes `subst_and_normalize_erasing_regions` and `try_subst_and_normalize_erasing_regions` to take  `EarlyBinder<T>` instead of `T`.

(related to rust-lang#105779)

This was suggested by `@BoxyUwU` in rust-lang#107753 (comment). After changing `type_of` to return `EarlyBinder`, there were several places where the binder was immediately skipped to call `tcx.subst_and_normalize_erasing_regions`, only for the binder to be reconstructed inside of that method.

r? `@BoxyUwU`
mukilan added a commit to mukilan/servo that referenced this pull request Jul 12, 2023
This requires fixing some nightly APIs that have been
changed in the latest nightly compiler.

For reference, the PR in rustc for the API change in
rustc crate: rust-lang/rust#107753

The tracking issue for the 'drain_filter' which is renamed
to 'extract_if': rust-lang/rust#43244

Signed-off-by: Mukilan Thiyagarajan <[email protected]>
mukilan added a commit to mukilan/servo that referenced this pull request Jul 12, 2023
This requires fixing some nightly APIs that have been
changed in the latest nightly compiler.

For reference, the PR in rustc for the API change in
rustc crate: rust-lang/rust#107753

The tracking issue for the 'drain_filter' which is renamed
to 'extract_if': rust-lang/rust#43244

Fixes servo#29467

Signed-off-by: Mukilan Thiyagarajan <[email protected]>
mukilan added a commit to mukilan/servo that referenced this pull request Jul 13, 2023
This requires fixing some nightly APIs that have been
changed in the latest nightly compiler.

For reference, the PR in rustc for the API change in
rustc crate: rust-lang/rust#107753

The tracking issue for the 'drain_filter' which is renamed
to 'extract_if': rust-lang/rust#43244

Fixes servo#29467

Signed-off-by: Mukilan Thiyagarajan <[email protected]>
mukilan added a commit to mukilan/servo that referenced this pull request Jul 14, 2023
This requires fixing some nightly APIs that have been
changed in the latest nightly compiler.

For reference, the PR in rustc for the API change in
rustc crate: rust-lang/rust#107753

The tracking issue for the 'drain_filter' which is renamed
to 'extract_if': rust-lang/rust#43244

Fixes servo#29467

Signed-off-by: Mukilan Thiyagarajan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants