Skip to content

Compilation errors when enabling the serde feature #122

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

Closed
wants to merge 1 commit into from

Conversation

fjarri
Copy link

@fjarri fjarri commented Jun 17, 2025

In my crate I was previously using derive-where for Debug and Clone, and tried to upgrade to 1.5 to use the new serde derivation as well. The test in the PR is a simplified extract from my code. The serde(bound) stuff is what I was hoping to get rid of.

The problem is, when just enabling the serde feature of derive-where, without any further changes, this code stops compiling. In this PR, you can see that just running cargo build --tests compiles fine, but cargo build --tests --features serde fails. The errors are:

error: Found unused `#[serde(...)]`
  --> tests/weird.rs:19:1
   |
17 | #[serde(crate = "serde_")]
   | ^

(in the original code, since there's no need for serde(crate), it's the same error but about serde(bound))

error[E0277]: the trait bound `ChainedProtocolError<Id, C>: Clone` is not satisfied
  --> tests/weird.rs:33:35
   |
31 | impl<Id, C> ProtocolError<Id> for ChainedProtocolError<Id, C>
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `ChainedProtocolError<Id, C>`

and

error[E0277]: `ChainedProtocolError<Id, C>` doesn't implement `Debug`
  --> tests/weird.rs:33:35
   |
31 | impl<Id, C> ProtocolError<Id> for ChainedProtocolError<Id, C>
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ChainedProtocolError<Id, C>` cannot be formatted using `{:?}`

Not sure what is happening here. I am going to try and find a workaround for my code.

Update: removing serde(bound) and switching to derive-where for serde fixes the compilation.

Update: Could this be cause by serde attribute being processed by derive-where even if Serialize/Deserialize are not derived with it?

@ModProg
Copy link
Owner

ModProg commented Jun 17, 2025

Update: Could this be cause by serde attribute being processed by derive-where even if Serialize/Deserialize are not derived with it?

yes that is most likly the case, I just checked and Found unused `#[serde(...)]` is an error from derive-where. @daxpedda we should maybe remove that validation and allow serde attributes even when there is no De/Serialize in the derive_where? I thought about checking if regular #[derive(serde::...)] is present, but that'd be too unreliable IMO.

@ModProg
Copy link
Owner

ModProg commented Jun 17, 2025

derive-where/src/error.rs

Lines 304 to 308 in 04c7056

/// Unsupported `serde(...)` without deriving `De/Serialize`.
#[cfg(feature = "serde")]
pub fn serde_without_serde(serde: Span) -> syn::Error {
syn::Error::new(serde, "Found unused `#[serde(...)]`")
}
is the error message

@daxpedda
Copy link
Collaborator

I'm trying to remember why I added this limitation in the first place. But I can't and I also am unable to come up with a good reason for this limitation.

I'm going to go ahead and just remove it.

@ModProg
Copy link
Owner

ModProg commented Jun 20, 2025

I'm trying to remember why I added this limitation in the first place. But I can't and I also am unable to come up with a good reason for this limitation.

I'm going to go ahead and just remove it.

probably to avoid user mistakes that don't result in compilation errors

@fjarri
Copy link
Author

fjarri commented Jun 20, 2025

This can probably be closed in favor of #123

@daxpedda
Copy link
Collaborator

I'm trying to remember why I added this limitation in the first place. But I can't and I also am unable to come up with a good reason for this limitation.
I'm going to go ahead and just remove it.

probably to avoid user mistakes that don't result in compilation errors

I can't come up with a scenarios where that can happen actually ...

@daxpedda daxpedda closed this Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants