-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Create rustc_type_ir
#79169
Create rustc_type_ir
#79169
Conversation
I haven't read through this yet, but I agree that we should rename |
8df14f2
to
2df3ece
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM! I'm on board with these changes as-is (only a fairly small subset of what will eventually get moved out), but I guess there are couple questions/concerns others might have:
- Should we be moving out more? I think no since we've discussed this in wg-traits meetings and decided any start is betting than nothing.
- Should things still have a shorthand, like
ty::INNERMOST
totylib::INNERMOST
or something like that? - Should this wait until after
rust_ty
MCP? Should this have an MCP itself? I think it's best to probably discuss this on Zulip.
And finally, let's r? @nikomatsakis, since I don't have r+ rights.
Another alternative would be to just make |
|
2df3ece
to
0cf5a8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start. Simple, but good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me
@bors r+ |
📌 Commit 0cf5a8a has been approved by |
☀️ Test successful - checks-actions |
…lan-DPC Fix typo in `DebruijnIndex` documentation Suggested in rust-lang#79169 (comment). r? `@lqd`
…lan-DPC Fix typo in `DebruijnIndex` documentation Suggested in rust-lang#79169 (comment). r? ``@lqd``
@LeSeulArtichaut @nikomatsakis this PR had some performance regressions in the const evaluation stress test (but not in other benchmarks). Is this change simply moving code around? I wonder if something that was previously being inlined is now not. |
Yes, this is literally just moving code around. Didn't expect a perf regression at all. I do imagine something isn't getting inlined anymore. |
Filed #80057 to add |
It's fairly common that PRs moving the code to a new crate regress performance a little.
That's often the reason. |
Refractor a few more types to `rustc_type_ir` In the continuation of rust-lang#79169, ~~blocked on that PR~~. This PR: - moves `IntVarValue`, `FloatVarValue`, `InferTy` (and friends) and `Variance` - creates the `IntTy`, `UintTy` and `FloatTy` enums in `rustc_type_ir`, based on their `ast` and `chalk_ir` equilavents, and uses them for types in the rest of the compiler. ~~I will split up that commit to make this easier to review and to have a better commit history.~~ EDIT: done, I split the PR in commits of 200-ish lines each r? `````@nikomatsakis````` cc `````@jackh726`````
Refractor a few more types to `rustc_type_ir` In the continuation of rust-lang#79169, ~~blocked on that PR~~. This PR: - moves `IntVarValue`, `FloatVarValue`, `InferTy` (and friends) and `Variance` - creates the `IntTy`, `UintTy` and `FloatTy` enums in `rustc_type_ir`, based on their `ast` and `chalk_ir` equilavents, and uses them for types in the rest of the compiler. ~~I will split up that commit to make this easier to review and to have a better commit history.~~ EDIT: done, I split the PR in commits of 200-ish lines each r? `````@nikomatsakis````` cc `````@jackh726`````
Decided to start small 😄
This PR creates a
rustc_type_ir
crate as part of the WG-Traits plan to create a shared type library.There already exists arustc_ty
crate, so I named the new craterustc_ty_library
. However I think it would make sense to rename the currentrustc_ty
to something else (e.g.rustc_ty_passes
) to free the name for this new crate.r? @jackh726