Skip to content

Conversation

@ShapelessCat
Copy link
Owner

Motivation

  • The derive macro previously required #[patchable] fields to be simple concrete types and failed for generic wrapper types, preventing deriving Patchable when a field uses a type parameter.
  • The change aims to support #[patchable] on fields whose types refer to generic parameters so generated state types and bounds are correct.

Description

  • Track patchable field types separately as patchable_field_types and replace the previous TypeUsage enum with a compact TypeUsage { used_in_keep: bool } to mark types referenced from kept fields.
  • Add collect_patchable_generic_params and is_generic_param_type helpers to detect generic parameters used by #[patchable] fields and include them in the generated state generics and bounds.
  • Generate patch field types using qualified projections for concrete patchable types (<#ty as crate::Patchable>::Patch) while emitting T::Patch for plain generic parameters, and add Patchable bounds for concrete field types.
  • Added a regression test in patchable/src/lib.rs that verifies #[patchable] works for a field with a generic wrapper type and ensured serde-compatible bounds are emitted as needed.

Testing

  • Ran cargo test which compiled the procedural macro and library and executed unit tests; all tests passed (6 passed unit tests, plus doctests).

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0214a905d2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +134 to +138
let patchable_generic_params = self.collect_patchable_generic_params();
for param in self.generics.type_params() {
if let Some(TypeUsage::Patchable) = self.preserved_types.get(&param.ident) {
let crate_root = &self.crate_path;
bounds.push(quote! { #param: #crate_root :: Patchable });
if patchable_generic_params.contains(&param.ident) {
bounds.push(quote! { #param: ::core::clone::Clone });
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid forcing Clone on generics only used by patchable fields

This adds T: Clone for any generic referenced inside a #[patchable] field type. That over‑constrains cases like struct Outer<T> { #[patchable] inner: Wrapper<T> } where Wrapper<T>: Patchable does not require T: Clone (e.g., PhantomData wrappers or manual Patchable impls whose Patch doesn’t depend on T). The new bound will make such valid uses fail to compile even though Wrapper<T>: Patchable is satisfied.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants