-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Implement Deref<Target = [T]> for [T; N], and fix const infer variable canonicalization (fix Deref<Target=[T; N]> ICEs)
#92652
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
Conversation
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 the only part of the PR I am somewhat unhappy with, but it works.
This comment has been minimized.
This comment has been minimized.
Deref<Target=[T; N]> on newtypes (attempt 2) (削除ここまで)Deref<Target = [T]> for [T; N], and fix const infer variable canonicalization (fix Deref<Target=[T; N]> ICEs) (追記ここまで)
5205639 to
595657d
Compare
Trait impls are insta-stable and I strongly disagree with a [T; N] -> [T] deref impl, regardless.
(click to expand my reasons - they're not really relevant outside of a FCP/RFC discussion)
The reason deref impls like Vec<T> -> [T] exist is because Vec<T> is like Box<[T]>, i.e. a pointer to [T].
Array unsizing (to slice) is in a different coercion category than deref coercions, and more flexible at that (e.g. Rc<[T; N]> -> Rc<[T]>, where a deref coercion couldn't give back an Rc) - I also don't see any reason to mix up the two too much (though long-term I would like to see generalized coercions).
Something this significant requires at least an FCP by @rust-lang/lang, if not an RFC.
The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Set({"src/doc/edition-guide"}) not skipped for "bootstrap::doc::EditionGuide" -- not in ["src/tools/tidy"]
Rustbook (x86_64-unknown-linux-gnu) - edition-guide
Building stage0 tool linkchecker (x86_64-unknown-linux-gnu)
Finished release [optimized] target(s) in 0.16s
core/primitive.array.html:336: broken link fragment `#method.to_ascii_uppercase` pointing to `core/primitive.array.html`
core/primitive.array.html:341: broken link fragment `#method.to_ascii_lowercase` pointing to `core/primitive.array.html`
core/primitive.array.html:1434: broken link - `core/slice::sort_by_key`
core/primitive.array.html:1519: broken link fragment `#method.sort_by_cached_key` pointing to `core/primitive.array.html`
number of HTML files scanned: 31972
number of HTML redirects found: 9914
number of links checked: 2162137
number of links ignored due to external: 89734
I misunderstood what exactly is happening here and after @eddyb's comment i am now also not in favor of [T; N] implementing Deref to [T] anymore, though I am not really opposed to it either 😆
will review #92640 and am in favor of only going forward with Deref for arrays in case an unrelated legitimate use case comes up
I agree with @eddyb that a Deref impl for [T; N] -> [T] is not obviously correct =)
Alright, I'll close this out then
Stash a const infer's type into the canonical var during canonicalization, so we can recreate the fresh const infer with that same type.
For example, given
[T; _]we know_is ausize. If we go from infer => canonical => infer, we shouldn't forget that variable is a usize.Fixes Inferring const parameters to wrapper type causes rustc to panic. #92626
Fixes ICE on Rust 1.51 with min const generics and Deref<Target=[T; N]> #83704
Implement
Dereffor[T; N]which allows us to remove some custom array unsizing logic that was leaking an inference variable during method selection.Fixes Calling slice method on newtype wrapper that implements
Deref<Target=[_; 1]>causes ICE #92637r? @lcnr