syn icon indicating copy to clipboard operation
syn copied to clipboard

Correct bounds processing for field types

Open dhardy opened this issue 6 years ago • 2 comments

The derive(HeapSize) example recursively calls heap_size_of_children on fields, but without adding the correct bound to the fields (f.ty: HeapSize).

Instead, trait bounds are added to generic parameters.

Why? I guess this is just not possible with macros yet, but these generic bounds are incorrect and confusing. Consider:

#[derive(HeapSize)]
struct MyStruct<'a, X: 'a> {
    a: u32,  // whoops, u32 does not support HeapSize and we did not check that
    _b: &'a X,  // who cares what X is; it doesn't affect us
}

I'm primarily asking because I'm trying to do something similar and can't see a correct way of implementing bounds. For generic parameters we should add something like #(field.ty.path): #bound to the where clauses; for non-generic parameters we have three options: (a) do the same (even though non generic), (b) check directly and fail if not met, (c) add meta-data specifying the constraint to the token stream.

dhardy avatar Mar 09 '18 12:03 dhardy

We have some experience with the approach you are thinking of from experimenting with it in Serde 0.7. We ended up quickly going back to the approach shown in the heapsize example in Serde 0.8. Neither one is always correct but the illustrated approach is what you want more often.

Private in public https://github.com/serde-rs/serde/issues/435

#[derive(Heapsize)]
pub struct Public<T> {
    private: Private<T>,
}

#[derive(Heapsize)]
struct Private<T>(T);

If Public's impl expands to impl<T> Heapsize for Public<T> where Private<T>: Heapsize then we have the private type in the public interface which fails to compile.

Overflow evaluating requirements https://github.com/serde-rs/serde/issues/441

#[derive(Heapsize)]
struct A<T> {
    t: PhantomData<T>,
    b: Option<Box<B<T>>>,
}

#[derive(Heapsize)]
struct B<T> {
    t: PhantomData<T>,
    a: Option<Box<A<T>>>,
}

Each of these structs would implement Heapsize only if the other implements Heapsize, which triggers an "overflow evaluating the requirement A<T>: Heapsize".

Lifetime deduplication inference is broken https://github.com/serde-rs/serde/issues/443

In many subtle and confusing ways. Here is the simplest example, which maybe you could work around with some effort but not in the general case.

#[derive(Heapsize)]
struct S<'a, 'b, T: 'a + 'b> {
    a: &'a T,
    b: &'b T,
}

// generated
impl<'a, 'b, T: 'a + 'b> Heapsize for S<'a, 'b, T>
where
    &'a T: Heapsize,
    &'b T: Heapsize,
{...}

As of rustc 1.26.0-nightly the generated impl fails to compile even if you have impl<'a, T> Heapsize for &'a T.

dtolnay avatar Mar 09 '18 17:03 dtolnay

No free lunch then — sounds like getting this correct is going to take more work.

It would be nice to see a comment about this in the example a bit (maybe just a link here) because it confused me why you would add those bounds.

dhardy avatar Mar 10 '18 07:03 dhardy