Helper trait hack const generics ICE #61383
Comments
varkor
added
A-const-generics
|
The discussion link is broken. Also, it'd be great if we could minimise this :) |
|
@varkor I've updated the discussion link. This is a minified version of the ICE: trait FooImpl<const IS_ZERO: bool>{}
impl FooImpl<{0u8==0u8}> for () {} |
Why exactly? This would only affect us if Default it were const today, no? But it seems not to be const: const fn foo() -> [u8; 32] {
Default::default()
}Gives: |
|
@est31 The thing is, for #49147 originally proposed relaxing this What we need is therefore a further extension to the concept, discussed in #49147 by @eddyb, to accept any expression that can be repeatedly evaluated at runtime. |
|
@HadrienG2 oh interesting, yeah we don't require it to be Copy. However, this is not the same as the thing @eddyb talked about. Lowering Consider a type It turns out that even if the type itself implements Your comment @HadrienG2 made me realize that the snippet in the issue description won't ever be correct. But fortunately there is this simple solution (not requiring #49147): impl<T,const N:usize> FooImpl<{0u8!=0u8}> for [T;N]
where
T:Default,
{
fn default_impl()->Self{
let ret: Self = unsafe { core::mem::uninitialized() };
let ret_slice: &mut [_] = ret;
for e in ret_slice.iter_mut() {
*e = Default::default();
}
ret
}
}It would probably be useful to have a function in libstd that wraps above code in a generic fashion, taking a closure |
|
I suspect that as written, your snippet has enough opportunities for UB to give @RalfJung a heart attack. But I like its overall logic, this is how I typically go about initializing arrays of primitive types myself. Given a bit of massaging with pointers instead of references, |
|
Note that the generic utility that you seek could take the form of an array-friendly form of |
You mean implementing |
|
Well, I'm not sure what exactly it should look like yet...
...but now that const generics are around, we can at least talk about it, which was not an option before. A PR would probably be frowned upon at this point in time, though, for the reasons outlined by @Centril in #60466 . Shall we take this half-baked idea to internals for consolidation? |
Yeah that'd probably be the best place to discuss it. It's kinda off-topic to this thread. Saying this as the person who has started the discussion :). |
…=oli-obk Fix an ICE with a const argument in a trait This goes some way towards fixing rust-lang#61383 (the reduced test case is fixed).
…=oli-obk Fix an ICE with a const argument in a trait This goes some way towards fixing rust-lang#61383 (the reduced test case is fixed).
…=oli-obk Fix an ICE with a const argument in a trait This goes some way towards fixing rust-lang#61383 (the reduced test case is fixed).
|
Okay, @varkor's PR #61409 has fixed the first minified example. The code in the issue description still ICEs. Minifying the error gives: fn f<const N: usize>()->[();N]{
[(); N]
} |
|
Oh I just realized that the current ICE it hits is probably the same as #61336. |
|
As of current 1.37.0-nightly (2887008 2019-06-12), the code snippet I pasted above doesn't ICE any more but instead gives a) a missing copy error and if that one is fixed by adding a trait bound, the However, since then I've realized that it won't ever work because we don't require T to be copy and instead should call #![feature(const_generics)]
trait Foo {
fn foo() -> Self;
}
impl<T, const N: usize> Foo for [T; N]
where
Self:FooImpl<{N==0}>
{
fn foo()->Self{
Self::default_impl()
}
}
trait FooImpl<const IS_ZERO:bool>{
fn default_impl()->Self;
}
impl<T> FooImpl<{0u8==0u8}> for [T;0] {
fn default_impl()->Self{
[]
}
}
impl<T,const N:usize> FooImpl<{0u8!=0u8}> for [T;N]
where
T:Default,
{
fn default_impl()->Self{
unsafe {
use std::mem::MaybeUninit;
let mut res = MaybeUninit::<Self>::uninit();
let res_mut_ptr = res.as_mut_ptr();
for i in 0 .. N {
*(res_mut_ptr.offset(i as isize) as * mut T) = T::default();
}
res.assume_init()
}
}
}I hope this doesn't trigger any UB cases but @RalfJung may differ. Anyways, that code snippet compiles without errors so I guess everything is fine now. |
|
That's almost right, but the index calculation is fatally flawed... You are offseting at array type so you are offsetting by It should be fn default_impl()->Self{
unsafe {
use std::mem::MaybeUninit;
let mut res = MaybeUninit::<Self>::uninit();
let res_mut_ptr = res.as_mut_ptr() as *mut T;
for i in 0 .. N {
*res_mut_ptr.add(i) = T::default();
}
res.assume_init()
}
}This is why we need raw slice access methods... |
|
@RalfJung lol I haven't thought about that one. I have used to convert everything into a slice but I think that's very UB-y for partially uninitialized data. After fixing it, trying to use it t gives an error now: #![feature(const_generics)]
trait Foo {
fn foo() -> Self;
}
impl<T, const N: usize> Foo for [T; N]
where
Self:FooImpl<{N==0}>
{
fn foo()->Self{
Self::default_impl()
}
}
trait FooImpl<const IS_ZERO:bool>{
fn default_impl()->Self;
}
impl<T> FooImpl<{0u8==0u8}> for [T;0] {
fn default_impl()->Self{
[]
}
}
impl<T,const N:usize> FooImpl<{0u8!=0u8}> for [T;N]
where
T:Default,
{
fn default_impl()->Self{
unsafe {
use std::mem::MaybeUninit;
let mut res = MaybeUninit::<Self>::uninit();
let res_mut_ptr = res.as_mut_ptr() as *mut T;
for i in 0 .. N {
*res_mut_ptr.add(i) = T::default();
}
res.assume_init()
}
}
}
fn main() {
let v: [u8; 64] = Foo::foo();
let v_slice: &[u8] = &v;
println!("{:?}", v_slice);
}@varkor is this intended? Will this one day be implemented? |
|
Btw, I've found another ICE with code very similar to this one, but decided to file a separate issue to not get this one into scope creep: #61806 |
It's not intended, but I'm also not so surprised that it fails. There are still some issues we need to resolve regarding evaluation/unification of const generics. |
|
@varkor should I file an issue for it? Is there an issue I can track? |
|
@est31: I don't think there's a specific tracking issue; it's worth opening one, so we have a test case. |
|
@varkor I'm est31, not estebank :). Will file an issue! |
|
@est31: I shouldn't rely on autocorrect to fix my GitHub mentions 😅 |
|
OK it's filed: #61935 |
In #60466 (comment), @rodrimati1992 shared a snippet to implement a trait only for numbers > 0. After a few modifications done by me, it gives you an ICE (playground):
Compiler error message
The text was updated successfully, but these errors were encountered: