Another [Pre-RFC] Custom DSTs

[kennytm]

Continuing the discussion from [Pre-RFC] Custom DSTs:

As promised here’s another take of custom DST, specified via reducing to a built-in one.

struct MatMeta {
    width: usize,
    height: usize,
    stride: usize,
}

// Define the DST
dyn type Mat<T>(regular: MatMeta) = [T];

impl<T> dst::Regular for Mat<T> {
    // Converts the metadata of the DST's to that of [T]'s.
    fn reduce_with_metadata(meta: MatMeta) -> usize {
        if meta.height == 0 { 
            0 
        } else { 
            (meta.height - 1) * meta.stride + meta.width
        }
    }
}

• Rendered

• Trivial fixes in https://github.com/kennytm/rfcs/pull/2 please.

Unlike all previous attempts which requires user to provide size_of_val and align_of_val, I think this approach is more suitable for Rust, since

1. it automatically handles Drop
2. it automatically handles Send and Sync
3. it needs less unsafe

Also, from the experience of impl Trait for .. ↦ auto trait Trait, I don’t think separating the metadata of the DST into an impl block is a good idea (only japaric’s draft attach the metadata to the type, every other proposal so far provides the metadata as an associated type of a Referent/DynSized trait ).

[Ixrec]

How would this work for a type like CStr, where (unless I’m horribly mistaken) the length is not contained in the metadata but instead has to be computed by following the data pointer and then searching for the null terminator?

[kennytm]

It is written in the “rendered” document already.

dyn type CStr(inline) = [u8];
unsafe impl dst::Inline for CStr {
  unsafe fn reduce_with_ptr(p: *const ()) -> usize {
    strlen(p as *const c_char) + 1
  }
}

[mark-i-m]

FYI, there is a current DSTs RFC: https://github.com/rust-lang/rfcs/pull/2594

[Centril]

I left a boatload of comments on the PR.

[ubsan]

How does this support FAMs?

This seems like a much more unnecessarily complex proposal, and I feel as though it doesn’t fix as many problems as the proposal I put forward. I don’t agree with this direction at all - the lack of DynSized is a major issue, in my very strong opinion - panicking in size_of_val and align_of_val is a really, really bad choice. It also is not a very minimal proposal, introducing unnecessary complexity in Aligned, imo.

[kennytm]

1. The lack of DynSize is due to it already being separated into RFC 2310 and the two RFCs are entirely orthogonal. I’m not going to repeat the same text in this new RFC.

2. As long as there is no no_panic effect any custom DST is able make size_of_val and align_of_val panic.

   unsafe impl DynamicSized for Oops {
       fn size_of_val(&self) -> usize { panic!() }
   }
   

3. The main reason Aligned is introduced is because of field offset in DST structs, and you’ll need to handle this issue if you want to support Thin<dyn Trait>. I see this as an essential complexity to prevent accidental unsafety.

[ubsan]

1. 2310 is not an RFC.

2. I don’t think that’s a reasonable excuse for allowing size_of_val to panic instead of fixing it on the type system level.

3. I don’t see what requires Aligned at all.

[kennytm]

2310 is not an RFC.

• I don’t think that’s a reasonable excuse for allowing size_of_val to panic instead of fixing it on the type system level.

I agree DynSized is desired but we also need to cope with that 2310 was postponed in favor of linting. I’d prefer reopening 2310 over unnecessary bundling.

I don’t see what requires Aligned at all.

I’ve discussed this in https://github.com/kennytm/rfcs/blob/dyn-type-8/text/0000-dyn-type.md#reduction-and-regularinline-dst and also replied about it in Pre-eRFC: Let’s fix DSTs. Basically: you cannot override align_of_val in DynSized because the pointer is unknown, so you can only override align_of_metadata, and a thin pointer has no metadata, forcing it to be Aligned.

You could get rid of Aligned but then your Thin<dyn Trait> would be potentially unsafe to Deref.

[ubsan]

@kennytm I disagree strongly that this machinery is necessary. I’m strongly of the opinion that the RFC I’ve put forward is both simpler, and easier to use, especially for the kind of Rust programmer who is likely to write the kind of code that requires this.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.