Skip to content
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

Correct bounds processing for field types #370

Open
dhardy opened this issue Mar 9, 2018 · 2 comments
Open

Correct bounds processing for field types #370

dhardy opened this issue Mar 9, 2018 · 2 comments
Labels

Comments

@dhardy
Copy link
Contributor

dhardy commented Mar 9, 2018

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.

@dtolnay
Copy link
Owner

dtolnay commented Mar 9, 2018

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 serde-rs/serde#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 serde-rs/serde#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 serde-rs/serde#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.

@dhardy
Copy link
Contributor Author

dhardy commented Mar 10, 2018

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.

@dtolnay dtolnay added the docs label Apr 2, 2018
Repository owner locked and limited conversation to collaborators May 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants