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

Tracking issue for custom_derive feature #29644

Closed
aturon opened this issue Nov 5, 2015 · 38 comments · Fixed by #54271
Closed

Tracking issue for custom_derive feature #29644

aturon opened this issue Nov 5, 2015 · 38 comments · Fixed by #54271
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Nov 5, 2015

Treats #[derive(Anything)] like #[derive_Anything]. Tracking for stabilization.

@aturon aturon added T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Feature: Implemented in the nightly compiler and unstable. labels Nov 5, 2015
@eefriedman
Copy link
Contributor

There isn't really much incentive to stabilize this at the moment; this is basically only useful with plugins (and syntex, but the gate doesn't affect that usage).

@nrc
Copy link
Member

nrc commented Mar 31, 2016

Is there an RFC for this? I couldn't find one easily.

@eefriedman
Copy link
Contributor

No RFC; see #23137.

@nrc
Copy link
Member

nrc commented Mar 31, 2016

Do we know who uses custom_derive? It seems like a horrible hack.

(Thanks for the link @eefriedman)

@pnkfelix
Copy link
Member

wow look at the turnaround time on that PR! Proposed, r+'ed, and landed all in one day!

@jimmycuadra
Copy link
Contributor

Two crates that use custom_derive: serde_macros and diesel_codegen.

@eefriedman
Copy link
Contributor

Servo also uses it in a few places, for example https://github.com/servo/heapsize .

@nrc
Copy link
Member

nrc commented Mar 31, 2016

Thanks for the info. I think custom derive is something we should support, but not in the current form. I'll have a think about how it fits in to the procedural macro overhaul and write some kind of RFC.

callahad added a commit to callahad/ladaemon that referenced this issue Jun 24, 2016
This is necessary because stable Rust does not yet support custom #[derive]
implementations, which are needed for Serde's Serialize/Deserialize traits.

Serde has a macro package and compiler plugin which handle those, but compiler
plugins are *also* not availble in stable Rust, so we instead have to generate
code at build time using serde_codegen.

Bug for `custom_derive` feature: rust-lang/rust#29644

Bug for compiler plugins: rust-lang/rust#29597
callahad added a commit to callahad/ladaemon that referenced this issue Jun 26, 2016
This is necessary because stable Rust does not yet support custom #[derive]
implementations, which are needed for Serde's Serialize/Deserialize traits.

Serde has a macro package and compiler plugin which handle those, but compiler
plugins are *also* not availble in stable Rust, so we instead have to generate
code at build time using serde_codegen.

Bug for `custom_derive` feature: rust-lang/rust#29644

Bug for compiler plugins: rust-lang/rust#29597
@e-oz
Copy link

e-oz commented Jun 29, 2016

HOPE WILL NEVER DIE!

Can't use XML/JSON deserialization to structures, but I still hope...

😢

@nrc
Copy link
Member

nrc commented Aug 29, 2016

cc #35900

@nrc
Copy link
Member

nrc commented Sep 27, 2016

Nominating for deprecation now that we have macros 1.1 custom derive in place.

@nikomatsakis nikomatsakis added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Sep 30, 2016
@nikomatsakis
Copy link
Contributor

@rfcbot fcp close

The general consensus is that this is subsumed by macros 1.1, which offers a more general solution.

@rfcbot
Copy link

rfcbot commented Oct 5, 2016

Team member nikomatsakis has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.
Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot
Copy link

rfcbot commented Oct 22, 2016

All relevant subteam members have reviewed. No concerns remain.

@aturon
Copy link
Member Author

aturon commented Oct 22, 2016

🔔 This feature is undergoing its final comment period with disposition to deprecate. 🔔

@rfcbot
Copy link

rfcbot commented Nov 1, 2016

The final comment period is now complete.

@brson brson removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jan 11, 2017
@brson
Copy link
Contributor

brson commented Jan 11, 2017

This is waiting on ultimate removal.

Note that here @SergioBenitez says Rocket needs this feature.

@sfackler
Copy link
Member

custom_derive is more powerful in a couple of ways than Macros 1.1; Rocket may be depending on the ability to adjust/remove the struct definition?

@nrc
Copy link
Member

nrc commented Jan 11, 2017

I'm in favour of a longer than usual deprecation period. As well as being more powerful, people apparently continue to use old custom derive because they can have a single crate which implements both derives and proc macros, and until we have a more function proc macro 2.0 solution, using new custom derive means having two separate implementations.

@SergioBenitez
Copy link
Contributor

SergioBenitez commented Jan 12, 2017

Please see my argument against removal in #37128 (comment). I'm okay with deprecation as long as #38533 remains valid (users don't receive a warning) and I can #[allow(deprecated)] in my codegen crate.

@joshhansen
Copy link

I'm confused about the status of this deprecation. The nightly compiler says derive for custom traits will be removed in 1.15, but nightly is now in 1.16. Combined with @SergioBenitez's comments and Rocket's continued dependence on this feature, I'm pretty unsure whether to assume it will continue for the foreseeable future or whether it really is at risk of being removed any time now. Please advise.

@keeperofdakeys
Copy link
Contributor

The deprecation warning was written many months ago, given the above comments the warning should probably be bumped a few versions.

@keeperofdakeys
Copy link
Contributor

So regarding the deprecation warning, what will the next removal target be? 1.17 + 3 = 1.20 perhaps?

@nikomatsakis
Copy link
Contributor

@joshhansen My feeling is that we should remove this and I would definitely not advise writing new code that relies on it. That said, I don't think there is a tremendous hurry to do so; all things being equal I'd prefer not to break rocket, of course. Still, if this feature ever becomes an obstacle to doing something we would want, I would be inclined to remove it ASAP, so migrating is definitely a good idea. I heard something about @jseyfried working with @SergioBenitez to move over to the prototypes of the newer macro system that may be relevant here.

@e-oz
Copy link

e-oz commented Feb 2, 2017

I feel absolutely confused. You just announced it here: https://blog.rust-lang.org/2017/02/02/Rust-1.15.html
but compiler saying it's deprecated. Please explain me where I'm wrong.

@jseyfried
Copy link
Contributor

@e-oz We just stabilized and announced a different feature (macros 1.1, aka proc_macro) that replaces this feature ("legacy custom derive"). Now that we've have macros 1.1 custom derives, we're deciding when to remove legacy custom derives.

@e-oz
Copy link

e-oz commented Feb 2, 2017

I have to say it was awful idea to release new feature with exactly same name as old deprecated feature.

@jseyfried
Copy link
Contributor

The name of the new feature was proc_macro, or macros 1.1. Both the old feature and the new feature allow defining and using custom derives.

@jseyfried
Copy link
Contributor

Of course, in retrospect it would have been better to name the old feature something more specific than custom_derive.

@aturon
Copy link
Member Author

aturon commented Feb 2, 2017

I think the complaint was about the release announcement, which said we're stabilizing "custom derive". I agree that this is all a bit confusing, especially for those not following Rust development closely.

@e-oz
Copy link

e-oz commented Feb 2, 2017

Yes, that's what broke my whole app and gave me work for a couple of next days.

bors added a commit that referenced this issue Feb 6, 2017
[beta] Give a better error message for unknown derive messages

This PR improves the error message for unknown derive macros.

Currently unknown derives give the following error, which is very misleading:
```
`#[derive]` for custom traits is not stable enough for use. It is deprecated and will be removed in v1.15 (see issue #29644)
```

I'm currently working on a PR that will change this (#39442) to the following:
```
cannot find derive macro `Foo` in this scope
```

This PR backports the (as yet unmerged) error message to beta/1.16 (it's a pity that this is probably too late for 1.15).

r? @jseyfried
@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@cramertj
Copy link
Member

@SergioBenitez Are you still using this feature?

@SergioBenitez
Copy link
Contributor

@cramertj Yes. With the recent improvements to the proc_macro APIs and a few additional ones, however, I should be able to move Rocket to proc_macro soon.

@fbstj
Copy link
Contributor

fbstj commented Jul 16, 2018

@SergioBenitez are you still using this feature?

@SergioBenitez
Copy link
Contributor

Yes. The plan is to migrate away from custom_derive as soon as possible however. Current progress puts us as doing so by next release, or about a month.

@SergioBenitez
Copy link
Contributor

SergioBenitez commented Sep 17, 2018

Update: All of Rocket's derives are now implemented as proc-macros and will ship as such in the next official release, which should be within a couple of weeks time.

bors added a commit that referenced this issue Dec 7, 2018
Unsupport `#[derive(Trait)]` sugar for `#[derive_Trait]` legacy plugin attributes

This is a long deprecated unstable feature that doesn't mesh well with regular resolution/expansion.

How to fix broken code:
- The recommended way is to migrate to stable procedural macros - derives or attributes (https://doc.rust-lang.org/nightly/book/first-edition/procedural-macros.html).
- If that's not possible right now for some reason, you can keep code working with a simple mechanical replacement `#[derive(Legacy)]` -> `#[derive_Legacy]`.

Closes #29644

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.