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

Feature-gate #[derive_*] even when produced by macro expansion. #32671

Closed
wants to merge 1 commit into from

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Apr 1, 2016

Fixes #32655 by marking #[derive_Trait] attributes produced by the compiler from the
user-friendly #[derive(Trait)] syntax to allow unstable features, and gating on that.

@eddyb eddyb added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Apr 1, 2016
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

// FIXME(eddyb) #[allow_internal_unstable]
// doesn't actually work for some reason.

#[derive(Clone)] //#[derive_Clone]
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't get this to work, seems like the span_allows_unstable check isn't enough.
cc @pnkfelix @nrc

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't you need to also turn on the feature gate for #[derive_X] syntax? Is there even such a gate? namely #![feature(custom_derive)]

Copy link
Member Author

Choose a reason for hiding this comment

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

There is, custom_derive, but allow_internal_unstable is supposed to bypass the actual feature gating (i.e. any feature can be used inside an #[allow_internal_unstable] macro).
This works with, e.g. #[thread_local] static FOO: usize = 0;, but not #[derive_Clone].

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, nevermind then, I thought it looked at the actual features enabled in the macro-declaring crate.

@alexcrichton
Copy link
Member

Crater report

@nikomatsakis
Copy link
Contributor

@eddyb based on the conversation with @erickt, if we can make this issue a warning -- even better, it'd be great if we could suggest a cargo update when see syntex in use -- that'd be good.

@eddyb eddyb mentioned this pull request Apr 7, 2016
@eddyb eddyb removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Apr 19, 2016
@bors
Copy link
Contributor

bors commented Apr 28, 2016

☔ The latest upstream changes (presumably #32791) made this pull request unmergeable. Please resolve the merge conflicts.

@eddyb eddyb closed this Apr 28, 2016
@eddyb eddyb deleted the derive_ branch April 28, 2016 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants