Externalize derive macro implementation

[vic1707]

Hi, I'm currently trying to make a crate built on top of serde to try to implement a relatively naive idea for struture versioning regarding #1137 as a POC (and hopefully a suitable solution).

For ease of use (both as the developper and user of the crate) I'd like my crate to be able to benefit from serde's derive implementation that is thankfully in a separate function that could be exported.

This would allow my crate (and other ones of the same kind) to be 100% compatible with all of serde derive's attribute and features by manually running the derive function and modifying the AST with quote and syn.

This would allow crate developers to inject their features direclty on the Serialize or Deserialize implementation outputed by serde_derive, kinda like what I'm doing:

--- usage.expanded.initial.rs	2024-06-28 23:42:09
+++ usage.expanded.modified.rs	2024-06-28 23:42:09
@@ -4,7 +4,7 @@
 use std::prelude::rust_2021::*;
 #[macro_use]
 extern crate std;
-use serde_derive::Deserialize;
+use my_serde::Deserialize;
 struct Foo {
     name: String,
     age: u8,
@@ -24,6 +24,11 @@
         where
             __D: _serde::Deserializer<'de>,
         {
+            'my_serde: {
+                {
+                    ::std::io::_print(format_args!("HI!!!\n"));
+                };
+            }
             #[allow(non_camel_case_types)]
             #[doc(hidden)]
             enum __Field {

original file being:

/* Clippy config */
#![allow(dead_code)]
/* Dependencies */
use serde_derive::Deserialize;

#[derive(Deserialize)]
struct Foo {
    name: String,
    age: u8,
    #[serde(default)]
    place_holder: String,
}

fn main() {}

This is, I think, kinda related to #2021.

To do so I pretty much replicated the serde_derive_internals situation.

If this change is ok to you (if its not I can simply use my fork for the time being), I'd like to have your opinion on how you want me to fix the current clippy issues (the lib file currently being the same as for serde_derive_internals) Simply add a bunch of #[allow()] on the lib.rs, try my best to fix every one of them, alternative ?

Notes

English isn't my primary language, I'm sorry if I made mistakes, if anything sounds bad or if I'm hard to understand 😓.

[vic1707]

Just released my code for versioning here if you want to take a look at what this PR allows ^^ https://github.com/vic1707/serde-versioning

btw, if you have time, I'd love to hear your opinion the project

[vic1707]

May I ask for your review @dtolnay ?

[oli-obk]

The issues raised in https://github.com/serde-rs/serde/pull/2021 are not really resolved by this.

I think this is a feature needed commonly enough that we could probably quickly figure out a small robust rust language feature to allow you to call proc macros from other proc macros. I think that would be better in general

[oli-obk]

Going to close this as I don't see it landing in this form any time soon.

[vic1707]

I think this is a feature needed commonly enough that we could probably quickly figure out a small robust rust language feature to allow you to call proc macros from other proc macros. I think that would be better in general

I also thought of a simple feature gate, but AFAIK

[lib]
proc-macro = true

cannot be set via by feature gate, at least not last time I checked, but it would be really nice to get that ! I also tried to enable it conditionally via the build.rs but it didn't work which is why I resulted to duplicating the serde_derive_internals solution. (I'm trying to find the sources and issue about that without luck for now, will update if I ever find them again)

Do you have other ideas I could explore ?

[oli-obk]

For now you can create a git repo with this repo as a submodule and create your crate that way. It'll be on you to update then, but that was already true. The main problem with those approaches is keeping in sync with the serde version for the internal API that the derives use

[oli-obk]

I don't know what would be needed on the rustc side to support invoking proc macros from other proc macros, but it doesn't seem insurmountable if the lang team agrees this is desirable

[vic1707]

For now you can create a git repo with this repo as a submodule and create your crate that way.

Haha, I honestly didn't even thought about submodules, thanks for the idea!

---

The main problem with those approaches is keeping in sync with the serde version for the internal API that the derives use

Yup but that's a burden for libraries developers not for serde maintainers right ? Lib devs would write down which version of serde is supported for each versions of their crates just like some already do when depending on/extending serde_derive themselves (thinking of crates like nutype which, I think, could face similar sync issues)? And no matter how I'm able to run serde_derive manually I will be facing those issues right?

(Not trying to get the PR opened again, just genuinely trying to understand, I probably am missing a point 😄)

---

I don't know what would be needed on the rustc side to support invoking proc macros from other proc macros, but it doesn't seem insurmountable if the lang team agrees this is desirable

Don't know either, hopefully I can find the issues/RFC back and link this PR or ask questions about it 🙃

[oli-obk]

Yup but that's a burden for libraries developers not for serde maintainers right ?

Oh yea I meant problems for you if you go with the submodule approach and potentially users of your crate.

Though it may be automatable in a cronjob to generate and publish the crate whenever a new serde_derive is released

[vic1707]

Oh, I thought you were saying that for serde and the PR itself, yup it could become a pain for me and others if serde changes too much.

I didn't saw it as an issue for me as in #1137 dtolnay said that if an extension of serde becomes a popular/viable solution it could get merged directly in serde and was hoping I could one day get that chance 🙃.

Thanks for the explanations and your time 👍

[vic1707]

Hi, sorry to bother you again 🙇

about

For now you can create a git repo with this repo as a submodule and create your crate that way.

how do you see this working ? My project https://github.com/vic1707/serde-versioning was already working using git url for the dependency, but couldn't publish to crates due to that. I tried many approaches to get a working version publishable on crates.io but can't get one to work properly 😓.

I successfully added my fork as a submodule at the root, pointing the dependency to the local path works fine but I can't publish to crates.io since the newly made crate doesn't exist there (expected, same as using the git url). So I tried multiple solutions using symlinks from and to various folders but none of them satisfies cargo build 😓.

Do you have an idea on how I can get it to work so I can do https://github.com/vic1707/serde-versioning/issues/2 😅 ?

[oli-obk]

I mean a literal git submodule, not a git dependency. Git submodules are not something cargo understands or cares about, it treats them as just normal files

[vic1707]

Yeah thats what I tried but the files weren't getting used during publish, I finally found a way to get it to work properly (see here if interested) 🥳

---

Would you be opened to merging half of this PR (in a separate PR ofc) ? I would like to merge the creation of the implementation subfolder in serde_derive and move the root files there. Meaning its the same content as this PR minus the new crate serde_derive_implementation.

Merging this tiny change, apart from making the serde_derive source a bit cleaner (in my opinion) would allow me close my fork as I can live without serde_derive_implementation using the submodule solution and directly depend on serde's HEAD commit, making everything a bit cleaner.

I understand this can be considered a selfish request so I'll wait for your answer before doing any of that.

[oli-obk]

You don't need that. You can point a module to any file, so your crate can have a module that is defined wherever the current files are

[vic1707]

It's what I tried but since serde_derive and the whole serde repo contains a lots of symlinks (vscode was fine with that but cargo always failed to build), and imports being crate or $crate in macros there was always something wrong. It may very well be a skill issue on my side tho.

The current working solution is

1. a git submodule pointing to my fork of serde, stored in the root of my new project.
2. a symlink from ../serde/serde_derive/src/implementation to my project src folder
3. a symlink from ../serde/serde_derive/src/internals to my project src folder

.
├── serde # submodule pointing to https://github.com/vic1707/serde - branch `derive-implementation-crate` 
└── src
    ├── attributes.rs
    ├── compile_error.rs
    ├── deserialize_impl.rs
    ├── implementation -> ../serde/serde_derive/src/implementation
    ├── internals -> ../serde/serde_derive/src/internals/
    ├── lib.rs
    └── utils.rs

Which means that to crates.io the implementation and internals modules are pretty much mine and gets pushed when publishing. These two modules depends on the $crate so I pretty much have to put the modules at the right place so it works.

It might not be pretty or the most optimal solution but I wasn't able to find another way yet, depending on serde_derive_implementation crate from the serde fork never worked due to the symlink. putting the whole serde submodule as a dependency of my project didn't work due to cargo wanting to pull crates.io serde instead of the fork, or cargo complaining for nested workspaces.

[vic1707]

After re-re-re-...-re reading your comment I think I finally understood what you were talking about

#[path = "../serde/serde_derive_implementation/src/mod.rs"]
mod serde_derive_implementation;

and using it like

use crate::serde_derive_implementation::de::expand_derive_deserialize;

Is this what you had in mind?

Unfortunately, due to the symlinks and crate::/$crate:: it causes cargo build to fail

...
error[E0433]: failed to resolve: could not find `implementation` in the crate root
    --> src/../serde/serde_derive_implementation/src/fragment.rs:21:17
     |
21   |           $crate::implementation::fragment::Fragment::Block(quote!($($tt)*))
     |                   ^^^^^^^^^^^^^^ could not find `implementation` in the crate root
     |
    ::: src/../serde/serde_derive_implementation/src/ser.rs:1039:13
     |
1039 | /             quote_block! {
1040 | |                 let #let_mut __serde_state = _serde::Serializer::serialize_map(
1041 | |                     __serializer,
1042 | |                     _serde::__private::None)?;
1043 | |                 #(#serialize_fields)*
1044 | |                 _serde::ser::SerializeMap::end(__serde_state)
1045 | |             }
     | |_____________- in this macro invocation
     |
     = note: this error originates in the macro `quote_block` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this enum
    -->  src/../serde/serde_derive_implementation/src/ser.rs:1:1
     |
1    + use crate::serde_derive_implementation::fragment::Fragment;
     |
...


error: could not compile `serde-versioning` (lib) due to 72 previous errors; 16 warnings emitted

[vic1707]

But using

#[path = "../serde/serde_derive/src/implementation/mod.rs"]
mod implementation;
#[path = "../serde/serde_derive/src/internals/mod.rs"]
mod internals;

I can get cargo build to work but not cargo publish despite adding "serde/**/*" to the include list of the Cargo.toml 🤔. I'll continue searching in this direction. But if it works too I'll still ask if it's possible to make the half-PR as having the implementation folder/module be part of main serde would allow me to not depend on a fork 😅 (else I'd have to make 7 #[path = ".../XX"] mod XX; one for each module at the root of serde_derive)

[oli-obk]

else I'd have to make 7 #[path = ".../XX"] mod XX; one for each module at the root of serde_derive)

That seems not too bad to me. If your crate becomes important enough to support it from serde-rs we can always revisit

[vic1707]

Ok then, will work with that for now then 👍