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

Comments

Projects
None yet
@aturon
Member

aturon commented Nov 5, 2015

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

@eefriedman

This comment has been minimized.

Contributor

eefriedman commented Nov 10, 2015

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

This comment has been minimized.

Member

nrc commented Mar 31, 2016

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

@eefriedman

This comment has been minimized.

Contributor

eefriedman commented Mar 31, 2016

No RFC; see #23137.

@nrc

This comment has been minimized.

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

This comment has been minimized.

Member

pnkfelix commented Mar 31, 2016

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

@jimmycuadra

This comment has been minimized.

Contributor

jimmycuadra commented Mar 31, 2016

Two crates that use custom_derive: serde_macros and diesel_codegen.

@eefriedman

This comment has been minimized.

Contributor

eefriedman commented Mar 31, 2016

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

@nrc

This comment has been minimized.

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

Add serde_codegen and build script for stable Rust
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

Add serde_codegen and build script for stable Rust
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

This comment has been minimized.

e-oz commented Jun 29, 2016

HOPE WILL NEVER DIE!

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

😢

@nrc

This comment has been minimized.

Member

nrc commented Aug 29, 2016

cc #35900

@nrc

This comment has been minimized.

Member

nrc commented Sep 27, 2016

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

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 5, 2016

@rfcbot fcp close

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

@rfcbot

This comment has been minimized.

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

This comment has been minimized.

rfcbot commented Oct 22, 2016

All relevant subteam members have reviewed. No concerns remain.

@aturon

This comment has been minimized.

Member

aturon commented Oct 22, 2016

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

@rfcbot

This comment has been minimized.

rfcbot commented Nov 1, 2016

The final comment period is now complete.

@brson

This comment has been minimized.

Contributor

brson commented Jan 11, 2017

This is waiting on ultimate removal.

Note that here @SergioBenitez says Rocket needs this feature.

@sfackler

This comment has been minimized.

Member

sfackler commented Jan 11, 2017

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

joshhansen commented Jan 21, 2017

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

This comment has been minimized.

Contributor

keeperofdakeys commented Jan 21, 2017

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

@keeperofdakeys

This comment has been minimized.

Contributor

keeperofdakeys commented Feb 2, 2017

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

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Feb 2, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

Contributor

jseyfried commented Feb 2, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

Contributor

jseyfried commented Feb 2, 2017

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

This comment has been minimized.

Contributor

jseyfried commented Feb 2, 2017

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

@aturon

This comment has been minimized.

Member

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

This comment has been minimized.

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 4, 2017

Auto merge of #39444 - keeperofdakeys:derive-error, r=jseyfried
[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

bors added a commit that referenced this issue Feb 6, 2017

Auto merge of #39444 - keeperofdakeys:derive-error, r=jseyfried
[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

@lawliet89 lawliet89 referenced this issue Feb 9, 2017

Closed

Re-implement Telegram Support #17

3 of 3 tasks complete

@SimonSapin SimonSapin referenced this issue Feb 19, 2017

Open

Tracking issue for 1.0.0 tracking issues #39954

9 of 28 tasks complete

@SergioBenitez SergioBenitez referenced this issue May 19, 2017

Open

Compile with stable Rust #19

7 of 12 tasks complete
@cramertj

This comment has been minimized.

Member

cramertj commented Jan 17, 2018

@SergioBenitez Are you still using this feature?

@SergioBenitez

This comment has been minimized.

Contributor

SergioBenitez commented Jan 19, 2018

@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

This comment has been minimized.

Contributor

fbstj commented Jul 16, 2018

@SergioBenitez are you still using this feature?

@SergioBenitez

This comment has been minimized.

Contributor

SergioBenitez commented Jul 16, 2018

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

This comment has been minimized.

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

Auto merge of #54271 - petrochenkov:nolegder, r=eddyb,alexcrichton
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

@bors bors closed this in #54271 Dec 7, 2018

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