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

Remove some explicit self.infcx for FnCtxt, which already derefs into InferCtxt #99615

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Remove some explicit self.infcx for FnCtxt, which already derefs into InferCtxt #99615

merged 1 commit into from
Jul 26, 2022

Conversation

compiler-errors
Copy link
Member

The use of self.infcx.method_on_infcx vs self.method_on_infcx when self is a FnCtxt is a bit inconsistent, so I'm moving some self.infcx usages I found to just use autoderef

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 22, 2022
@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(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 Jul 22, 2022
@compiler-errors
Copy link
Member Author

compiler-errors commented Jul 22, 2022 •
edited
Loading

r? @lcnr who has been thinking about FnCtxt very recently -- do you have a strong opinion about the autoderef behavior of FnCtxt?

I guess alternatively, we could remove the autoderef behavior of FnCtxt altogether, lol.

@rust-highfive rust-highfive assigned lcnr and unassigned fee1-dead Jul 22, 2022
@fee1-dead
Copy link
Member

I think fcx.tcx could be a call to a function: fcx.tcx() otherwise I think we could use some verbosity.

@compiler-errors
Copy link
Member Author

compiler-errors commented Jul 23, 2022 •
edited
Loading

i could see us having FnCtxt deref to Inherited but have Inherited no longer deref to InferCtxt.

We could then either have tcx() has a fn like @fee1-dead said on fcx (if i'm interpreting what u said correctly) or just store tcx: TyCtxt<'tcx> again in the FnCtxt struct (like i don't think it's that expensive to store it in two places, infcx and fcx)

edit: Actually I think the changes from fcx -> fcx.infcx is much more involved. So many places in fcx use the implicit deref, lol.

lcnr
lcnr reviewed Jul 25, 2022
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

🤔 it might be worth it to remove the deref to InferCtxt from the FnCtxt but i think generally this doesn't matter too much.

r=me after my nit

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs Outdated Show resolved Hide resolved
@compiler-errors
Copy link
Member Author

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Jul 26, 2022

📌 Commit aaa9989 has been approved by lcnr

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 Jul 26, 2022
@JohnTitor JohnTitor mentioned this pull request Jul 26, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2022
@bors
Auto merge of rust-lang#99745 - JohnTitor:rollup-lvrie64, r=JohnTitor
b629c85 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#98211 (Implement `fs::get_path` for FreeBSD.)
 - rust-lang#99353 (Slightly improve mismatched GAT where clause error)
 - rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value)
 - rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`)
 - rust-lang#99711 (Remove reachable coverage without counters)
 - rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions)
 - rust-lang#99720 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2744c0e into rust-lang:master Jul 26, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 26, 2022
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Aug 24, 2022
@bors
Auto merge of rust-lang#99745 - JohnTitor:rollup-lvrie64, r=JohnTitor
1fb02e0 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#98211 (Implement `fs::get_path` for FreeBSD.)
 - rust-lang#99353 (Slightly improve mismatched GAT where clause error)
 - rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value)
 - rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`)
 - rust-lang#99711 (Remove reachable coverage without counters)
 - rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions)
 - rust-lang#99720 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@compiler-errors compiler-errors deleted the remove-some-explicit-infcx branch August 11, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr left review comments

Assignees

@lcnr lcnr

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
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
6 participants
@compiler-errors @rust-highfive @fee1-dead @bors @lcnr @rustbot