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

() not Serializable #88

Open
Sushisource opened this issue Mar 31, 2022 · 6 comments
Open

() not Serializable #88

Sushisource opened this issue Mar 31, 2022 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Sushisource
Copy link

Hey all, starting playing around with this, looks very cool.

Ran into the problem in the title almost right away though, which was rather surprising. I assume just an oversight, but would be nice to have.

Additionally when I first tried writing a generic Serializable struct, I added bounds to the generic for T: Serializable which I would expect to be necessary, but the macro seems to break on that (and if you remove it, appears to silently enforce it). It'd be good to allow that, or have a more explicit error.

Thanks!

@Sushisource Sushisource changed the title () not Serializeable () not Serializable Mar 31, 2022
@arendjr arendjr added enhancement New feature or request good first issue Good for newcomers labels Mar 31, 2022
@emschwartz
Copy link
Contributor

Based on @arendjr adding the good first issue label, I think he's saying that yes this is indeed an oversight :) Thanks for reporting the issue!

@sagacity
Copy link
Contributor

sagacity commented Jul 4, 2022

I'll pick this up.

@sagacity
Copy link
Contributor

sagacity commented Jul 4, 2022

@Sushisource could you share the struct definition that wasn't working for you?

@Sushisource
Copy link
Author

Even just this will do it:

use fp_bindgen::prelude::*;

#[derive(Serializable)]
struct Foo {
    hi: ()
}

Results in:

λ  cargo build
   Compiling rust-scratch v0.1.0 (/mnt/chonky/dev/rust-scratch)
error: proc-macro derive panicked
 --> src/main.rs:9:10
  |
9 | #[derive(Serializable)]
  |          ^^^^^^^^^^^^
  |
  = help: message: Only value types are supported. Incompatible type in struct field: Field { attrs: [], vis: Inherited, ident: Some(Ident { ident: "hi", span: #0 bytes(158..160) }), colon_token: Some(Colon), ty: Tuple(TypeTuple { paren_token: Paren, elems: [] }) }

@sagacity
Copy link
Contributor

sagacity commented Jul 6, 2022

@Sushisource thanks for sharing. I'm currently doing some work to allow unit types to be used in return types (e.g. Result<(), Error>). Do you have a use case for having unit fields in a struct? Otherwise I'm not sure we'd want to support that.

@Sushisource
Copy link
Author

Yeah, the use case is a generic struct like this:

use fp_bindgen::prelude::*;

#[derive(Serializable)]
struct Foo<T> {
    hi: T
}

Where some reification of that struct might reasonably want to use the unit type because it happens not to care about populating that field meaningfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants