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

Splitting up rustc_typeck::check::fn_ctxt::FnCtxt #77085

Closed
1 of 3 tasks
Nicholas-Baron opened this issue Sep 23, 2020 · 3 comments · Fixed by #77808
Closed
1 of 3 tasks

Splitting up rustc_typeck::check::fn_ctxt::FnCtxt #77085

Nicholas-Baron opened this issue Sep 23, 2020 · 3 comments · Fixed by #77808
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Nicholas-Baron
Copy link
Contributor

Nicholas-Baron commented Sep 23, 2020

Branching this from #60302.

Originally, mod.rs for rustc_typeck::check was over 6,000 lines. I was able to push that to around 1,100 lines (PR #76906).

However, in the process, I found that FnCtxt was rather long, with 13 fields in the struct and some rather long impls attached. The sum total is 3,200 lines, which effectively means that the // ignore-tidy-filelength comment was moved from mod.rs to fn_ctxt.rs.

The goal of this issue is to

  1. Make fn_ctxt.rs smaller, at minimum less than 3,000 lines by
  2. Moving some of its functionality into "sub-structs" that will be in separate files

Unknowns to solve:

  • How do the 13 fields group into nice sub-structs
  • What to name the above mentioned sub-structs
  • How to achieve this refactoring without changing the interface of FnCtxt
    Solution: Cannot do
@Nicholas-Baron
Copy link
Contributor Author

@rustbot modify labels to +T-compiler and +C-cleanup.

@rustbot rustbot added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 23, 2020
@Nicholas-Baron
Copy link
Contributor Author

The third unknown is probably impossible. All fields (except err_count_on_creation) can be accessed from outside the file (pub(super)).

So the interface must change.

@Nicholas-Baron
Copy link
Contributor Author

@rustbot modify labels to +E-needs-mentor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants