Allow #[serde(crate = "...")] to override extern crate serde

[sgrif]

This is intended to be used by other crates which provide their own proc
macros and use serde internally. Today there's no consistent way to put
#[derive(Deserialize)] on a struct that consistently works, since
crates may be using either features = ["derive"] or relying on
serde_derive separately.

Even if we assume that everyone is using features = ["derive"],
without this commit, any crate which generates
#[derive(serde::Deserialize)] forces its consumers to put serde in
their Cargo.toml, even if they aren't otherwise using serde for
anything.

Examples of crates which suffer from this in the real world are
tower-web and swirl.

With this feature, it's expected that these crates would have pub extern crate serde; in some accessible path, and add
#[serde(serde_path = "that_crate::wherever::serde")] anywhere they
place serde's derives. Those crates would also have to derive
that_crate::whatever::serde::Deserialize, or use the macros
explicitly beforehand.

The test for this is a little funky, as it's testing this in a way that
is not the intended use case, or even one we want to support. It has its
own module which re-exports all of serde, but defines its own
Serialize and Deserialize traits. We then test that we generated
impls for those traits, instead of serde's. The only other way to test
this would be to create a new test crate which does not depend on serde,
but instead depends on serde_derive and a third crate which publicly
re-exports serde. This feels like way too much overhead for a single
test case, hence the funky test given.

I didn't see anywhere in this repo to document this attribute, so I
assume the docs will have to be done as a separate PR to a separate
repo.

Fixes #1487

[sgrif]

CI failures are unrelated to this, the compiletest version used here isn't compatible with the latest nightly. That should probably get locked to a single nightly version, and have the general test suite on nightly made an allowed failure

[dtolnay]

Thanks!

Based on #1487 (comment) and Carl's response in #1487 (comment), it sounds like this implementation won't work for the tower-web use case. I have not looked at the details, but it seems their crate that re-exports serde is not accessed by their macro in a way that can be imported via use. Would you be interested in exploring the other implementation, that avoids use?

CI failures are unrelated to this, the compiletest version used here isn't compatible with the latest nightly. That should probably get locked to a single nightly version, and have the general test suite on nightly made an allowed failure

Sorry about the red build. This was not a nightly incompatibility that pinning a compiler version would fix. Compiletest released a broken version that fails to compile on all nightly and stable compilers and has since been yanked -- Manishearth/compiletest-rs#164. We run compiletest on nightly but in the mode that does not use any nightly features, so it should not be necessary to pin the compiler version for building compiletest. Separately, I want to get notified when rustc regresses our error messages in nightly.

[sgrif]

I'm happy to explore other options, but since it's a much more complex impl, I'd prefer to verify that this actually doesn't work before going that route. use certainly can be used in a const scope, even for modules defined in that scope (though I'd assume this is re-exported from tower-web somewhere since the whole point is to not assume serde exists in the crate the code is generated for)

[sgrif]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=9d6190759caef2812249a4b366868318 is an example

[dtolnay]

const _CONST: () = {
    use __tw::codegen::serde as _serde;
    impl _serde::Serialize for T { ... }
};

@carllerche -- Carl, would you be able to find out whether this implementation is sufficient for what tower-web needs? We are generating a use line and it seems to work even when __tw is a relative path not reachable from the crate root, at least in 2018 edition.

@sgrif -- how about in 2015 edition? Neither use fake_serde nor use self::fake_serde appears to work. The tower-web and tower-web-macros crates are currently on 2015 edition so any paths they emit would behave with 2015 semantics. If tower-web is stuck on 2015 edition for any reason then that may pose a problem. Or maybe they will need to use a shim crate set to 2018 edition through which they can inject a 2018 style path into the generated code.

If we go with this implementation, it would be best if we could future-proof by inserting some gadget into the generated code that only compiles if _serde::Serialize and fake_serde::Serialize refer to the same thing. That leaves us free to skip the use line in the future if necessary, which otherwise might be a breaking change.

[carllerche]

@dtolnay

I am on mobile. I will have to check later.

I have never gotten use statments to work in the context of the anonymous const scope. It is possible that this is due to 2015 and it works fine in 2018.

I plan on shifting tower-web to 2018 only in the next breaking release, so it is fine on my end to only support 2018.

[sgrif]

@dtolnay use works in 2015 for modules defined outside of the const, which is really all we care about here

[dtolnay]

@sgrif, based on tower-web-macros/src/derive/extract.rs, the problematic case is:

const _TOWER_WEB_CONST: () = {
    extern crate tower_web as __tw;

    const _SERDE_CONST: () = {
        use ??? as _serde; // __tw::codegen::serde

        ...
    };

    ...
};

which I have not found how to make work in 2015 edition.

[sgrif]

Thanks, that helps me understand better. I'll investigate further and update the implementation or comment with how to make the current implementation work in 2015 with that limitation

[dtolnay]

I think based on #1499 (comment) we can just target 2018 edition downstream crates for this feature. We don't necessarily need something that works in a 2015 crate as part of the initial implementation.

The remaining work is:

• Please make the generated code refer to at least one thing directly via the given path, rather than through _serde. That way we avoid replacing all uses of _serde up front but make it backward compatible to do it later if necessary for 2015 support.

• Please rename the attribute to serde(crate = "...") which I suggested in Allow specifying serde path and avoid injecting extern crate serde as _serde. #1487. I don't like the stutter in serde_path:

  #[serde(serde_path = "...")]
    ^^^^^ ^^^^^

[sgrif]

Sounds good, will do

…

On Fri, Mar 22, 2019 at 4:38 PM David Tolnay ***@***.***> wrote: I think based on #1499 (comment) <#1499 (comment)> we can just target 2018 edition downstream crates for this feature. We don't necessarily need something that works in a 2015 crate as part of the initial implementation. The remaining work is: - Please make the generated code refer to at least one thing directly via the given path, rather than through _serde. That way we avoid replacing all uses of _serde up front but make it backward compatible to do it later if necessary for 2015 support. - Please rename the attribute to serde(crate = "...") which I suggested in #1487 <#1487>. I don't like the stutter in serde_path: #[serde(serde_path = "...")] ^^^^^ ^^^^^ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1499 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdWK310EgiVUXf2tM6m5pgDmyf5f-D3ks5vZVs9gaJpZM4b6zkS> .

-- Thanks, Sean Griffin

[carllerche]

Does use in nested const scopes work in 2018? In general, is using improved in 2018 then?

[sgrif]

Yes, because `self` is no longer needed in 2018

…

On Fri, Mar 22, 2019 at 6:11 PM Carl Lerche ***@***.***> wrote: Does use in nested const scopes work in 2018? In general, is using improved in 2018 then? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1499 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdWK1pphwfNlOrC45Ta-jQgZVKS0zPFks5vZXEWgaJpZM4b6zkS> .

-- Thanks, Sean Griffin

[sgrif]

@dtolnay I have changed the attr name to crate and made the first two quote! blocks in #[derive(Serialize)] access through the given path directly rather than _serde

[dtolnay]

Thanks!

I applied a similar change in the Deserialize impl so that we don't end up breaking code like this either:

#[derive(Deserialize)]
#[serde(path = "...")]
struct ...

[dtolnay]

Released in serde_derive 1.0.90.