allow eq constraints on associated constants by JulianKnodt · Pull Request #8764...
source link: https://github.com/rust-lang/rust/pull/87648
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Some changes occurred in src/tools/clippy.
Some changes occurred in src/tools/rustfmt.
r? @estebank
(rust-highfive has picked a reviewer for you, use r? to override)
This comment has been hidden.
I have been musing about "just" adding a TyKind::Const and removing all the duplication we have around const generics vs type generics. Since this would also simplify our representation of associated const vs associated type as visible in this PR, maybe that's something we should pursue?
I'll give this PR a closer review on monday, but my fingers are still itching for TyKind::Const
Hmmmm, I think if it's a useful tool to simplify implementations, then it's probably useful, but at the same time I can't really picture what a TyKind::Const means. Is it an instance of a type which is const? Or does it represent an abstract const instantiation of a type?
This comment has been hidden.
force-pushed the const_eq_constrain
branch
2 times, most recently
from
33ffbf3
to
58c97a9
6 months ago
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
Hmmmm, I think if it's a useful tool to simplify implementations, then it's probably useful, but at the same time I can't really picture what a
TyKind::Constmeans. Is it an instance of a type which is const? Or does it represent an abstract const instantiation of a type?
it is essentially the equivalent of a built-in struct Foo<T, const C: T>; without the struct/Adt.
think about it this way: before const generics we had typenum, this is essentially typenum but directly supported by the compiler. typenum encodes constants in the type system, with TyKind::Const we do the same thing. We don't have to support any surface syntax for this, so users can't impl Trait for 42 {}, but the type system would support constants as first class types.
If we want to we could even represent it as such. So not even adding a TyKind::Const, but having Foo be a lang item in libcore that all const generics desugar to, but I fear that is just as much of a problem as making bool an enum bool { true, false } lang item. Nice from a cleanliness perspective, but impractical and a performance problem.
I've never heard of typenum, I've not been working on rustc long enough to have seen it 
. If you're going to implement it, tag me in the changes and I'll hold off on this change until that's done?
I'm going to do an MCP, and if it is declined, we go with your PR, if not, I'll implement TyKind::Const
Note: typenum is just a regular crate on crates.io, nothing rustc specific. It was the go-to workaround for lack of const generics and still is the go-to workaround for lack of const_evaluatable_checked.
Sounds good, please cc in the MCP as well, I'm interested in those changes as well!
The latest upstream changes (presumably #86860) made this pull request unmergeable. Please resolve the merge conflicts.
r? @oli-obk
Changes to llvm and cargo looks like not intended, bad rebase?
Finished benchmarking commit (7bc7be8): comparison url.
Summary: This change led to moderate relevant mixed results 
in compiler performance.
- Moderate improvement in instruction counts (up to -1.2% on
incr-unchangedbuilds ofdeeply-nested-async check) - Small regression in instruction counts (up to 0.3% on
fullbuilds ofdiesel check)
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.
Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.
@rustbot label: +perf-regression
Changes to llvm and cargo looks like not intended, bad rebase?
I don't see any submodule changes... is the mobile app hiding them from me?
hmmm well the cargo lock file would fail CI if I didn't update @klensy, so I just put it in, not sure what caused it to change
Oof yea, the app just bails on me on these links... I guess no more reviews in the app :(
Please revert the submodule changes and the lockfile changes!
Do I have permission to revert the PR? I can push to the branch again but since it's already merged
edit: or what exactly should I do?
You can revert the PR locally and open a new PR with the revert.
Revert all commits, squash the reverts, and only keep the reverts of the submodules and cargo lock, the functional changes can stay
This PR changed the crate rustdoc-json-types which is a public API (nightly, but still) without increasing the format version and without pinging a rustdoc maintainer. I've send #93132 to fix the format version.
cc @CraftSpider (as rustdoc json maintainer)
@Urgau simply bumping version is wrong, as this pr should be reverted.
rustdoc hitted too, btw.
Or it expected changes? Idk.
The changes are expected from this PR, yes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK