Skip to content

Commit

Permalink
Auto merge of #54271 - petrochenkov:nolegder, r=eddyb,alexcrichton
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Dec 7, 2018
2 parents 5182cc1 + 8ab115c commit a2fb99b
Show file tree
Hide file tree
Showing 19 changed files with 30 additions and 213 deletions.
2 changes: 0 additions & 2 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,6 @@ where
}
});

let whitelisted_legacy_custom_derives = registry.take_whitelisted_custom_derives();
let Registry {
syntax_exts,
early_lint_passes,
Expand Down Expand Up @@ -955,7 +954,6 @@ where
crate_loader,
&resolver_arenas,
);
resolver.whitelisted_legacy_custom_derives = whitelisted_legacy_custom_derives;
syntax_ext::register_builtins(&mut resolver, syntax_exts, sess.features_untracked().quote);

// Expand all macros
Expand Down
16 changes: 0 additions & 16 deletions src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ pub struct Registry<'a> {

#[doc(hidden)]
pub attributes: Vec<(String, AttributeType)>,

whitelisted_custom_derives: Vec<ast::Name>,
}

impl<'a> Registry<'a> {
Expand All @@ -77,7 +75,6 @@ impl<'a> Registry<'a> {
lint_groups: FxHashMap::default(),
llvm_passes: vec![],
attributes: vec![],
whitelisted_custom_derives: Vec::new(),
}
}

Expand Down Expand Up @@ -130,19 +127,6 @@ impl<'a> Registry<'a> {
}));
}

/// This can be used in place of `register_syntax_extension` to register legacy custom derives
/// (i.e. attribute syntax extensions whose name begins with `derive_`). Legacy custom
/// derives defined by this function do not trigger deprecation warnings when used.
pub fn register_custom_derive(&mut self, name: ast::Name, extension: SyntaxExtension) {
assert!(name.as_str().starts_with("derive_"));
self.whitelisted_custom_derives.push(name);
self.register_syntax_extension(name, extension);
}

pub fn take_whitelisted_custom_derives(&mut self) -> Vec<ast::Name> {
::std::mem::replace(&mut self.whitelisted_custom_derives, Vec::new())
}

/// Register a macro of the usual kind.
///
/// This is a convenience wrapper for `register_syntax_extension`.
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1588,7 +1588,6 @@ pub struct Resolver<'a, 'b: 'a> {
macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
macro_defs: FxHashMap<Mark, DefId>,
local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
pub whitelisted_legacy_custom_derives: Vec<Name>,
pub found_unresolved_macro: bool,

/// List of crate local macros that we need to warn about as being unused.
Expand Down Expand Up @@ -1922,7 +1921,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
macro_defs,
local_macro_def_scopes: FxHashMap::default(),
name_already_seen: FxHashMap::default(),
whitelisted_legacy_custom_derives: Vec::new(),
potentially_unused_imports: Vec::new(),
struct_constructors: Default::default(),
found_unresolved_macro: false,
Expand Down
102 changes: 3 additions & 99 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,18 @@ use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX, DefIndex,
use rustc::hir::def::{Def, NonMacroAttrKind};
use rustc::hir::map::{self, DefCollector};
use rustc::{ty, lint};
use syntax::ast::{self, Name, Ident};
use syntax::ast::{self, Ident};
use syntax::attr;
use syntax::errors::DiagnosticBuilder;
use syntax::ext::base::{self, Determinacy};
use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
use syntax::ext::base::{MacroKind, SyntaxExtension};
use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
use syntax::ext::hygiene::{self, Mark};
use syntax::ext::tt::macro_rules;
use syntax::feature_gate::{self, feature_err, emit_feature_err, is_builtin_attr_name, GateIssue};
use syntax::feature_gate::EXPLAIN_DERIVE_UNDERSCORE;
use syntax::feature_gate::{feature_err, is_builtin_attr_name, GateIssue};
use syntax::fold::{self, Folder};
use syntax::parse::parser::PathStyle;
use syntax::parse::token::{self, Token};
use syntax::ptr::P;
use syntax::symbol::{Symbol, keywords};
use syntax::tokenstream::{TokenStream, TokenTree, Delimited, DelimSpan};
use syntax::util::lev_distance::find_best_match_for_name;
use syntax_pos::{Span, DUMMY_SP};
use errors::Applicability;
Expand Down Expand Up @@ -194,10 +190,6 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
ret.into_iter().next().unwrap()
}

fn is_whitelisted_legacy_custom_derive(&self, name: Name) -> bool {
self.whitelisted_legacy_custom_derives.contains(&name)
}

fn visit_ast_fragment_with_placeholders(&mut self, mark: Mark, fragment: &AstFragment,
derives: &[Mark]) {
let invocation = self.invocations[&mark];
Expand Down Expand Up @@ -240,79 +232,6 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
ImportResolver { resolver: self }.resolve_imports()
}

// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>, allow_derive: bool)
-> Option<ast::Attribute> {
if !allow_derive {
return None;
}

// Check for legacy derives
for i in 0..attrs.len() {
let name = attrs[i].name();

if name == "derive" {
let result = attrs[i].parse_list(&self.session.parse_sess, |parser| {
parser.parse_path_allowing_meta(PathStyle::Mod)
});

let mut traits = match result {
Ok(traits) => traits,
Err(mut e) => {
e.cancel();
continue
}
};

for j in 0..traits.len() {
if traits[j].segments.len() > 1 {
continue
}
let trait_name = traits[j].segments[0].ident.name;
let legacy_name = Symbol::intern(&format!("derive_{}", trait_name));
if !self.builtin_macros.contains_key(&legacy_name) {
continue
}
let span = traits.remove(j).span;
self.gate_legacy_custom_derive(legacy_name, span);
if traits.is_empty() {
attrs.remove(i);
} else {
let mut tokens = Vec::with_capacity(traits.len() - 1);
for (j, path) in traits.iter().enumerate() {
if j > 0 {
tokens.push(TokenTree::Token(attrs[i].span, Token::Comma).into());
}
tokens.reserve((path.segments.len() * 2).saturating_sub(1));
for (k, segment) in path.segments.iter().enumerate() {
if k > 0 {
tokens.push(TokenTree::Token(path.span, Token::ModSep).into());
}
let tok = Token::from_ast_ident(segment.ident);
tokens.push(TokenTree::Token(path.span, tok).into());
}
}
let delim_span = DelimSpan::from_single(attrs[i].span);
attrs[i].tokens = TokenTree::Delimited(delim_span, Delimited {
delim: token::Paren,
tts: TokenStream::concat(tokens).into(),
}).into();
}
return Some(ast::Attribute {
path: ast::Path::from_ident(Ident::new(legacy_name, span)),
tokens: TokenStream::empty(),
id: attr::mk_attr_id(),
style: ast::AttrStyle::Outer,
is_sugared_doc: false,
span,
});
}
}
}

None
}

fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
let (path, kind, derives_in_scope, after_derive) = match invoc.kind {
Expand Down Expand Up @@ -430,11 +349,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
feature_err(&self.session.parse_sess, "rustc_attrs", path.span,
GateIssue::Language, &msg).emit();
}
} else if name.starts_with("derive_") {
if !features.custom_derive {
feature_err(&self.session.parse_sess, "custom_derive", path.span,
GateIssue::Language, EXPLAIN_DERIVE_UNDERSCORE).emit();
}
} else if !features.custom_attribute {
let msg = format!("The attribute `{}` is currently unknown to the \
compiler and may have meaning added to it in the \
Expand Down Expand Up @@ -1218,14 +1132,4 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
self.define(module, ident, MacroNS, (def, vis, item.span, expansion));
}
}

fn gate_legacy_custom_derive(&mut self, name: Symbol, span: Span) {
if !self.session.features_untracked().custom_derive {
let sess = &self.session.parse_sess;
let explain = feature_gate::EXPLAIN_CUSTOM_DERIVE;
emit_feature_err(sess, "custom_derive", span, GateIssue::Language, explain);
} else if !self.is_whitelisted_legacy_custom_derive(name) {
self.session.span_warn(span, feature_gate::EXPLAIN_DEPR_CUSTOM_DERIVE);
}
}
}
7 changes: 0 additions & 7 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,16 +733,12 @@ pub trait Resolver {
fn next_node_id(&mut self) -> ast::NodeId;
fn get_module_scope(&mut self, id: ast::NodeId) -> Mark;
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item>;
fn is_whitelisted_legacy_custom_derive(&self, name: Name) -> bool;

fn visit_ast_fragment_with_placeholders(&mut self, mark: Mark, fragment: &AstFragment,
derives: &[Mark]);
fn add_builtin(&mut self, ident: ast::Ident, ext: Lrc<SyntaxExtension>);

fn resolve_imports(&mut self);
// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>, allow_derive: bool)
-> Option<Attribute>;

fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
Expand Down Expand Up @@ -771,15 +767,12 @@ impl Resolver for DummyResolver {
fn next_node_id(&mut self) -> ast::NodeId { ast::DUMMY_NODE_ID }
fn get_module_scope(&mut self, _id: ast::NodeId) -> Mark { Mark::root() }
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item> { item }
fn is_whitelisted_legacy_custom_derive(&self, _name: Name) -> bool { false }

fn visit_ast_fragment_with_placeholders(&mut self, _invoc: Mark, _fragment: &AstFragment,
_derives: &[Mark]) {}
fn add_builtin(&mut self, _ident: ast::Ident, _ext: Lrc<SyntaxExtension>) {}

fn resolve_imports(&mut self) {}
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>, _allow_derive: bool)
-> Option<Attribute> { None }
fn resolve_macro_invocation(&mut self, _invoc: &Invocation, _invoc_id: Mark, _force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
Err(Determinacy::Determined)
Expand Down
14 changes: 0 additions & 14 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,12 +1134,6 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
let (mut attr, mut traits, mut after_derive) = (None, Vec::new(), false);

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
true) {
attr = Some(legacy_attr_invoc);
return attrs;
}

attr = self.find_attr_invoc(&mut attrs, &mut after_derive);
traits = collect_derives(&mut self.cx, &mut attrs);
attrs
Expand All @@ -1156,12 +1150,6 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
let (mut attr, mut after_derive) = (None, false);

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
false) {
attr = Some(legacy_attr_invoc);
return attrs;
}

attr = self.find_attr_invoc(&mut attrs, &mut after_derive);
attrs
});
Expand Down Expand Up @@ -1623,15 +1611,13 @@ impl<'feat> ExpansionConfig<'feat> {
}

feature_tests! {
fn enable_quotes = quote,
fn enable_asm = asm,
fn enable_custom_test_frameworks = custom_test_frameworks,
fn enable_global_asm = global_asm,
fn enable_log_syntax = log_syntax,
fn enable_concat_idents = concat_idents,
fn enable_trace_macros = trace_macros,
fn enable_allow_internal_unstable = allow_internal_unstable,
fn enable_custom_derive = custom_derive,
fn enable_format_args_nl = format_args_nl,
fn macros_in_extern_enabled = macros_in_extern,
fn proc_macro_hygiene = proc_macro_hygiene,
Expand Down
19 changes: 3 additions & 16 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,6 @@ declare_features! (
// Allows the use of custom attributes; RFC 572
(active, custom_attribute, "1.0.0", Some(29642), None),

// Allows the use of #[derive(Anything)] as sugar for
// #[derive_Anything].
(active, custom_derive, "1.0.0", Some(29644), None),

// Allows the use of rustc_* attributes; RFC 572
(active, rustc_attrs, "1.0.0", Some(29642), None),

Expand Down Expand Up @@ -530,6 +526,9 @@ declare_features! (
Some("subsumed by `#![feature(proc_macro_hygiene)]`")),
(removed, panic_implementation, "1.28.0", Some(44489), None,
Some("subsumed by `#[panic_handler]`")),
// Allows the use of `#[derive(Anything)]` as sugar for `#[derive_Anything]`.
(removed, custom_derive, "1.0.0", Some(29644), None,
Some("subsumed by `#[proc_macro_derive]`")),
);

declare_features! (
Expand Down Expand Up @@ -1287,8 +1286,6 @@ impl<'a> Context<'a> {
"unless otherwise specified, attributes \
with the prefix `rustc_` \
are reserved for internal compiler diagnostics");
} else if name.starts_with("derive_") {
gate_feature!(self, custom_derive, attr.span, EXPLAIN_DERIVE_UNDERSCORE);
} else if !attr::is_known(attr) {
// Only run the custom attribute lint during regular
// feature gate checking. Macro gating runs
Expand Down Expand Up @@ -1418,16 +1415,6 @@ pub const EXPLAIN_ALLOW_INTERNAL_UNSTABLE: &str =
pub const EXPLAIN_ALLOW_INTERNAL_UNSAFE: &str =
"allow_internal_unsafe side-steps the unsafe_code lint";

pub const EXPLAIN_CUSTOM_DERIVE: &str =
"`#[derive]` for custom traits is deprecated and will be removed in the future.";

pub const EXPLAIN_DEPR_CUSTOM_DERIVE: &str =
"`#[derive]` for custom traits is deprecated and will be removed in the future. \
Prefer using procedural macro custom derive.";

pub const EXPLAIN_DERIVE_UNDERSCORE: &str =
"attributes of the form `#[derive_*]` are reserved for the compiler";

pub const EXPLAIN_UNSIZED_TUPLE_COERCION: &str =
"unsized tuple coercion is not stable enough for use and is subject to change";

Expand Down
9 changes: 3 additions & 6 deletions src/libsyntax_ext/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ extern crate log;

mod diagnostics;

#[macro_use]
// for custom_derive
pub mod deriving;

mod asm;
mod assert;
mod cfg;
Expand All @@ -54,13 +50,14 @@ mod format;
mod format_foreign;
mod global_asm;
mod log_syntax;
mod trace_macros;
mod proc_macro_server;
mod test;
mod test_case;
mod trace_macros;

pub mod deriving;
pub mod proc_macro_decls;
pub mod proc_macro_impl;
mod proc_macro_server;

use rustc_data_structures::sync::Lrc;
use syntax::ast;
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/proc-macro/derive-still-gated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#[macro_use]
extern crate derive_a;

#[derive_A] //~ ERROR: attributes of the form `#[derive_*]` are reserved for the compiler
#[derive_A] //~ ERROR attribute `derive_A` is currently unknown
struct A;

fn main() {}
4 changes: 2 additions & 2 deletions src/test/run-pass-fulldeps/auxiliary/custom_derive_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ use rustc_plugin::Registry;

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_custom_derive(
reg.register_syntax_extension(
Symbol::intern("derive_TotalSum"),
MultiDecorator(box expand));

reg.register_custom_derive(
reg.register_syntax_extension(
Symbol::intern("derive_Nothing"),
MultiDecorator(box noop));
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass-fulldeps/custom-derive-partial-eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

// aux-build:custom_derive_partial_eq.rs
// ignore-stage1
#![feature(plugin, custom_derive)]
#![feature(plugin)]
#![plugin(custom_derive_partial_eq)]
#![allow(unused)]

#[derive(CustomPartialEq)] // Check that this is not a stability error.
#[derive_CustomPartialEq] // Check that this is not a stability error.
enum E { V1, V2 }

fn main() {}
Loading

0 comments on commit a2fb99b

Please sign in to comment.