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

internal refactoring: cleanup type relations #121159

Open
2 of 4 tasks
lcnr opened this issue Feb 15, 2024 · 2 comments
Open
2 of 4 tasks

internal refactoring: cleanup type relations #121159

lcnr opened this issue Feb 15, 2024 · 2 comments
Labels
A-technical-debt Area: Internal cleanup work A-typesystem Area: The type system C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Feb 15, 2024 •
edited
Loading

we should try to clean up our type relations further:

  • try to unify Equate and Sub to make them closer to nll::TypeRelating
  • merge Lub and Glb into the same (maybe generic) type relation, prolly just with a field
  • reuse more components, e.g. instantiating infer vars should be shared, not only generalize
  • completely merge Equate/Sub and nll:TypeRelating

I intend to look into this myself but would also appreciate some help. This is a quite involved and sometimes subtle area however and I don't have the capacity for in-depth mentoring rn
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 15, 2024
@lcnr lcnr changed the title internal refactoring: combine cleanups internal refactoring: cleanup type relations Feb 15, 2024
@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-typesystem Area: The type system E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. A-technical-debt Area: Internal cleanup work E-help-wanted Call for participation: Help is requested to fix this issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 15, 2024
@lcnr lcnr removed the E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. label Feb 15, 2024
@lcnr lcnr mentioned this issue Feb 17, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 19, 2024
@bors
Auto merge of rust-lang#121211 - lcnr:nll-relate-handle-infer, r=BoxyUwU
0395fa3 
deduplicate infer var instantiation

Having 3 separate implementations of one of the most subtle parts of our type system is not a good strategy if we want to maintain a sound type system ✨ while working on this I already found some subtle bugs in the existing code, so that's awesome 🎉 cc rust-lang#121159

This was necessary as I am not confident in my nll changes in rust-lang#119106, so I am first cleaning this up in a separate PR.

r? `@BoxyUwU`
@ShoyuVanilla
Copy link
Member

try to unify Equate and Sub to make them closer to nll::TypeRelating

It seems to be done by #121462, I guess?

@ShoyuVanilla
Copy link
Member

ShoyuVanilla commented Mar 11, 2024 •
edited
Loading

completely merge Equate/Sub and nll:TypeRelating

And nll::TypeRelating is moved into rustc_borrowck in #121321

@lcnr lcnr removed the E-help-wanted Call for participation: Help is requested to fix this issue. label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-technical-debt Area: Internal cleanup work A-typesystem Area: The type system C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ShoyuVanilla @lcnr @jieyouxu @rustbot