Skip to content

Commit 2a9b752

Browse files
petrochenkovpetrochenkov
committed
Move special treatment of derive(Copy, PartialEq, Eq) from expansion infrastructure to elsewhere
1 parent a457433 commit 2a9b752

17 files changed

+167
-108
lines changed

‎src/librustc/hir/lowering.rs

+16-2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ use syntax::ast;
6161
use syntax::ptr::P as AstP;
6262
use syntax::ast::*;
6363
use syntax::errors;
64+
use syntax::ext::base::SpecialDerives;
6465
use syntax::ext::hygiene::ExpnId;
6566
use syntax::print::pprust;
6667
use syntax::source_map::{respan, ExpnInfo, ExpnKind, DesugaringKind, Spanned};
@@ -176,6 +177,8 @@ pub trait Resolver {
176177
components: &[Symbol],
177178
is_value: bool,
178179
) -> (ast::Path, Res<NodeId>);
180+
181+
fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool;
179182
}
180183

181184
/// Context of `impl Trait` in code, which determines whether it is allowed in an HIR subtree,
@@ -1329,13 +1332,17 @@ impl<'a> LoweringContext<'a> {
13291332
}
13301333
}
13311334

1332-
fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
1335+
fn lower_attrs_extendable(&mut self, attrs: &[Attribute]) -> Vec<Attribute> {
13331336
attrs
13341337
.iter()
13351338
.map(|a| self.lower_attr(a))
13361339
.collect()
13371340
}
13381341

1342+
fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
1343+
self.lower_attrs_extendable(attrs).into()
1344+
}
1345+
13391346
fn lower_attr(&mut self, attr: &Attribute) -> Attribute {
13401347
// Note that we explicitly do not walk the path. Since we don't really
13411348
// lower attributes (we use the AST version) there is nowhere to keep
@@ -4028,7 +4035,14 @@ impl<'a> LoweringContext<'a> {
40284035
pub fn lower_item(&mut self, i: &Item) -> Option<hir::Item> {
40294036
let mut ident = i.ident;
40304037
let mut vis = self.lower_visibility(&i.vis, None);
4031-
let attrs = self.lower_attrs(&i.attrs);
4038+
let mut attrs = self.lower_attrs_extendable(&i.attrs);
4039+
if self.resolver.has_derives(i.id, SpecialDerives::PARTIAL_EQ | SpecialDerives::EQ) {
4040+
// Add `#[structural_match]` if the item derived both `PartialEq` and `Eq`.
4041+
let ident = Ident::new(sym::structural_match, i.span);
4042+
attrs.push(attr::mk_attr_outer(attr::mk_word_item(ident)));
4043+
}
4044+
let attrs = attrs.into();
4045+
40324046
if let ItemKind::MacroDef(ref def) = i.node {
40334047
if !def.legacy || attr::contains_name(&i.attrs, sym::macro_export) {
40344048
let body = self.lower_token_stream(def.stream());

‎src/librustc_resolve/lib.rs

+18-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ use rustc_metadata::cstore::CStore;
3838
use syntax::source_map::SourceMap;
3939
use syntax::ext::hygiene::{ExpnId, Transparency, SyntaxContext};
4040
use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
41-
use syntax::ext::base::SyntaxExtension;
42-
use syntax::ext::base::MacroKind;
41+
use syntax::ext::base::{SyntaxExtension, MacroKind, SpecialDerives};
4342
use syntax::symbol::{Symbol, kw, sym};
4443
use syntax::util::lev_distance::find_best_match_for_name;
4544

@@ -1685,6 +1684,12 @@ pub struct Resolver<'a> {
16851684
local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
16861685
unused_macros: NodeMap<Span>,
16871686
proc_macro_stubs: NodeSet,
1687+
/// Some built-in derives mark items they are applied to so they are treated specially later.
1688+
/// Derive macros cannot modify the item themselves and have to store the markers in the global
1689+
/// context, so they attach the markers to derive container IDs using this resolver table.
1690+
/// FIXME: Find a way for `PartialEq` and `Eq` to emulate `#[structural_match]`
1691+
/// by marking the produced impls rather than the original items.
1692+
special_derives: FxHashMap<ExpnId, SpecialDerives>,
16881693

16891694
/// Maps the `ExpnId` of an expansion to its containing module or block.
16901695
invocations: FxHashMap<ExpnId, &'a InvocationData<'a>>,
@@ -1813,6 +1818,12 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
18131818
fn definitions(&mut self) -> &mut Definitions {
18141819
&mut self.definitions
18151820
}
1821+
1822+
fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool {
1823+
let def_id = self.definitions.local_def_id(node_id);
1824+
let expn_id = self.definitions.expansion_that_defined(def_id.index);
1825+
self.has_derives(expn_id, derives)
1826+
}
18161827
}
18171828

18181829
impl<'a> Resolver<'a> {
@@ -2031,6 +2042,7 @@ impl<'a> Resolver<'a> {
20312042
struct_constructors: Default::default(),
20322043
unused_macros: Default::default(),
20332044
proc_macro_stubs: Default::default(),
2045+
special_derives: Default::default(),
20342046
current_type_ascription: Vec::new(),
20352047
injected_crate: None,
20362048
active_features:
@@ -2077,6 +2089,10 @@ impl<'a> Resolver<'a> {
20772089
}
20782090
}
20792091

2092+
fn has_derives(&self, expn_id: ExpnId, markers: SpecialDerives) -> bool {
2093+
self.special_derives.get(&expn_id).map_or(false, |m| m.contains(markers))
2094+
}
2095+
20802096
/// Entry point to crate resolution.
20812097
pub fn resolve_crate(&mut self, krate: &Crate) {
20822098
ImportResolver { resolver: self }.finalize_imports();

‎src/librustc_resolve/macros.rs

+31-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use rustc::{ty, lint, span_bug};
1414
use syntax::ast::{self, Ident, ItemKind};
1515
use syntax::attr::{self, StabilityLevel};
1616
use syntax::edition::Edition;
17-
use syntax::ext::base::{self, Indeterminate};
17+
use syntax::ext::base::{self, Indeterminate, SpecialDerives};
1818
use syntax::ext::base::{MacroKind, SyntaxExtension};
1919
use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
2020
use syntax::ext::hygiene::{self, ExpnId, ExpnInfo, ExpnKind};
@@ -203,8 +203,28 @@ impl<'a> base::Resolver for Resolver<'a> {
203203
(&mac.node.path, MacroKind::Bang, Vec::new(), false),
204204
InvocationKind::Derive { ref path, .. } =>
205205
(path, MacroKind::Derive, Vec::new(), false),
206-
InvocationKind::DeriveContainer { .. } =>
207-
return Ok(None),
206+
InvocationKind::DeriveContainer { ref derives, .. } => {
207+
// Block expansion of derives in the container until we know whether one of them
208+
// is a built-in `Copy`. Skip the resolution if there's only one derive - either
209+
// it's not a `Copy` and we don't need to do anything, or it's a `Copy` and it
210+
// will automatically knows about itself.
211+
let mut result = Ok(None);
212+
if derives.len() > 1 {
213+
let parent_scope = self.invoc_parent_scope(invoc_id, Vec::new());
214+
for path in derives {
215+
match self.resolve_macro_path(path, Some(MacroKind::Derive),
216+
&parent_scope, true, force) {
217+
Ok((Some(ref ext), _)) if ext.is_derive_copy => {
218+
self.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
219+
return Ok(None);
220+
}
221+
Err(Determinacy::Undetermined) => result = Err(Indeterminate),
222+
_ => {}
223+
}
224+
}
225+
}
226+
return result;
227+
}
208228
};
209229

210230
let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
@@ -234,6 +254,14 @@ impl<'a> base::Resolver for Resolver<'a> {
234254
);
235255
}
236256
}
257+
258+
fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool {
259+
self.has_derives(expn_id, derives)
260+
}
261+
262+
fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives) {
263+
*self.special_derives.entry(expn_id).or_default() |= derives;
264+
}
237265
}
238266

239267
impl<'a> Resolver<'a> {

‎src/libsyntax/ext/base.rs

+16-2
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,8 @@ pub struct SyntaxExtension {
595595
/// Built-in macros have a couple of special properties (meaning of `$crate`,
596596
/// availability in `#[no_implicit_prelude]` modules), so we have to keep this flag.
597597
pub is_builtin: bool,
598+
/// We have to identify macros providing a `Copy` impl early for compatibility reasons.
599+
pub is_derive_copy: bool,
598600
}
599601

600602
impl SyntaxExtensionKind {
@@ -640,6 +642,7 @@ impl SyntaxExtension {
640642
helper_attrs: Vec::new(),
641643
edition,
642644
is_builtin: false,
645+
is_derive_copy: false,
643646
kind,
644647
}
645648
}
@@ -683,6 +686,16 @@ pub type NamedSyntaxExtension = (Name, SyntaxExtension);
683686
/// Error type that denotes indeterminacy.
684687
pub struct Indeterminate;
685688

689+
bitflags::bitflags! {
690+
/// Built-in derives that need some extra tracking beyond the usual macro functionality.
691+
#[derive(Default)]
692+
pub struct SpecialDerives: u8 {
693+
const PARTIAL_EQ = 1 << 0;
694+
const EQ = 1 << 1;
695+
const COPY = 1 << 2;
696+
}
697+
}
698+
686699
pub trait Resolver {
687700
fn next_node_id(&mut self) -> ast::NodeId;
688701

@@ -699,6 +712,9 @@ pub trait Resolver {
699712
-> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;
700713

701714
fn check_unused_macros(&self);
715+
716+
fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool;
717+
fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives);
702718
}
703719

704720
#[derive(Clone)]
@@ -725,7 +741,6 @@ pub struct ExtCtxt<'a> {
725741
pub resolver: &'a mut dyn Resolver,
726742
pub current_expansion: ExpansionData,
727743
pub expansions: FxHashMap<Span, Vec<String>>,
728-
pub allow_derive_markers: Lrc<[Symbol]>,
729744
}
730745

731746
impl<'a> ExtCtxt<'a> {
@@ -745,7 +760,6 @@ impl<'a> ExtCtxt<'a> {
745760
directory_ownership: DirectoryOwnership::Owned { relative: None },
746761
},
747762
expansions: FxHashMap::default(),
748-
allow_derive_markers: [sym::rustc_attrs, sym::structural_match][..].into(),
749763
}
750764
}
751765

‎src/libsyntax/ext/expand.rs

+4-15
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::attr::{self, HasAttrs};
44
use crate::source_map::{dummy_spanned, respan};
55
use crate::config::StripUnconfigured;
66
use crate::ext::base::*;
7-
use crate::ext::proc_macro::{add_derived_markers, collect_derives};
7+
use crate::ext::proc_macro::collect_derives;
88
use crate::ext::hygiene::{ExpnId, SyntaxContext, ExpnInfo, ExpnKind};
99
use crate::ext::placeholders::{placeholder, PlaceholderExpander};
1010
use crate::feature_gate::{self, Features, GateIssue, is_builtin_attr, emit_feature_err};
@@ -209,7 +209,6 @@ pub enum InvocationKind {
209209
Derive {
210210
path: Path,
211211
item: Annotatable,
212-
item_with_markers: Annotatable,
213212
},
214213
/// "Invocation" that contains all derives from an item,
215214
/// broken into multiple `Derive` invocations when expanded.
@@ -360,8 +359,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
360359

361360
let mut item = self.fully_configure(item);
362361
item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
363-
let mut item_with_markers = item.clone();
364-
add_derived_markers(&mut self.cx, item.span(), &traits, &mut item_with_markers);
365362
let derives = derives.entry(invoc.expansion_data.id).or_default();
366363

367364
derives.reserve(traits.len());
@@ -370,11 +367,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
370367
let expn_id = ExpnId::fresh(self.cx.current_expansion.id, None);
371368
derives.push(expn_id);
372369
invocations.push(Invocation {
373-
kind: InvocationKind::Derive {
374-
path,
375-
item: item.clone(),
376-
item_with_markers: item_with_markers.clone(),
377-
},
370+
kind: InvocationKind::Derive { path, item: item.clone() },
378371
fragment_kind: invoc.fragment_kind,
379372
expansion_data: ExpansionData {
380373
id: expn_id,
@@ -383,7 +376,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
383376
});
384377
}
385378
let fragment = invoc.fragment_kind
386-
.expect_from_annotatables(::std::iter::once(item_with_markers));
379+
.expect_from_annotatables(::std::iter::once(item));
387380
self.collect_invocations(fragment, derives)
388381
} else {
389382
unreachable!()
@@ -569,13 +562,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
569562
}
570563
_ => unreachable!()
571564
}
572-
InvocationKind::Derive { path, item, item_with_markers } => match ext {
565+
InvocationKind::Derive { path, item } => match ext {
573566
SyntaxExtensionKind::Derive(expander) |
574567
SyntaxExtensionKind::LegacyDerive(expander) => {
575-
let (path, item) = match ext {
576-
SyntaxExtensionKind::LegacyDerive(..) => (path, item_with_markers),
577-
_ => (path, item),
578-
};
579568
if !item.derive_allowed() {
580569
return fragment_kind.dummy(span);
581570
}

‎src/libsyntax/ext/proc_macro.rs

+2-33
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
use crate::ast::{self, ItemKind, Attribute, Mac};
2-
use crate::attr::{mark_used, mark_known, HasAttrs};
2+
use crate::attr::{mark_used, mark_known};
33
use crate::errors::{Applicability, FatalError};
44
use crate::ext::base::{self, *};
55
use crate::ext::proc_macro_server;
66
use crate::parse::{self, token};
77
use crate::parse::parser::PathStyle;
8-
use crate::symbol::{sym, Symbol};
8+
use crate::symbol::sym;
99
use crate::tokenstream::{self, TokenStream};
1010
use crate::visit::Visitor;
1111

12-
use rustc_data_structures::fx::FxHashSet;
1312
use rustc_data_structures::sync::Lrc;
14-
use syntax_pos::hygiene::{ExpnInfo, ExpnKind};
1513
use syntax_pos::{Span, DUMMY_SP};
1614

1715
const EXEC_STRATEGY: proc_macro::bridge::server::SameThread =
@@ -217,32 +215,3 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>)
217215
});
218216
result
219217
}
220-
221-
crate fn add_derived_markers<T: HasAttrs>(
222-
cx: &mut ExtCtxt<'_>, span: Span, traits: &[ast::Path], item: &mut T
223-
) {
224-
let (mut names, mut pretty_name) = (FxHashSet::default(), String::new());
225-
for (i, path) in traits.iter().enumerate() {
226-
if i > 0 {
227-
pretty_name.push_str(", ");
228-
}
229-
pretty_name.push_str(&path.to_string());
230-
names.insert(unwrap_or!(path.segments.get(0), continue).ident.name);
231-
}
232-
233-
let span = span.fresh_expansion(cx.current_expansion.id, ExpnInfo::allow_unstable(
234-
ExpnKind::Macro(MacroKind::Derive, Symbol::intern(&pretty_name)), span,
235-
cx.parse_sess.edition, cx.allow_derive_markers.clone(),
236-
));
237-
238-
item.visit_attrs(|attrs| {
239-
if names.contains(&sym::Eq) && names.contains(&sym::PartialEq) {
240-
let meta = cx.meta_word(span, sym::structural_match);
241-
attrs.push(cx.attribute(meta));
242-
}
243-
if names.contains(&sym::Copy) {
244-
let meta = cx.meta_word(span, sym::rustc_copy_clone_marker);
245-
attrs.push(cx.attribute(meta));
246-
}
247-
});
248-
}

‎src/libsyntax/ext/tt/macro_rules.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,8 @@ pub fn compile(
429429
}
430430
}
431431

432+
let is_builtin = attr::contains_name(&def.attrs, sym::rustc_builtin_macro);
433+
432434
SyntaxExtension {
433435
kind: SyntaxExtensionKind::LegacyBang(expander),
434436
span: def.span,
@@ -440,7 +442,8 @@ pub fn compile(
440442
deprecation: attr::find_deprecation(&sess, &def.attrs, def.span),
441443
helper_attrs: Vec::new(),
442444
edition,
443-
is_builtin: attr::contains_name(&def.attrs, sym::rustc_builtin_macro),
445+
is_builtin,
446+
is_derive_copy: is_builtin && def.ident.name == sym::Copy,
444447
}
445448
}
446449

‎src/libsyntax/feature_gate.rs

-5
Original file line numberDiff line numberDiff line change
@@ -1329,11 +1329,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
13291329
/*opt*/ attributes(name1, name2, ...)"),
13301330
Ungated),
13311331

1332-
(sym::rustc_copy_clone_marker, Whitelisted, template!(Word), Gated(Stability::Unstable,
1333-
sym::rustc_attrs,
1334-
"internal implementation detail",
1335-
cfg_fn!(rustc_attrs))),
1336-
13371332
(sym::rustc_allocator, Whitelisted, template!(Word), Gated(Stability::Unstable,
13381333
sym::rustc_attrs,
13391334
"internal implementation detail",

‎src/libsyntax_ext/deriving/clone.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ use crate::deriving::generic::*;
33
use crate::deriving::generic::ty::*;
44

55
use syntax::ast::{self, Expr, GenericArg, Generics, ItemKind, MetaItem, VariantData};
6-
use syntax::attr;
7-
use syntax::ext::base::{Annotatable, ExtCtxt};
6+
use syntax::ext::base::{Annotatable, ExtCtxt, SpecialDerives};
87
use syntax::ptr::P;
98
use syntax::symbol::{kw, sym, Symbol};
109
use syntax_pos::Span;
@@ -36,7 +35,8 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt<'_>,
3635
match annitem.node {
3736
ItemKind::Struct(_, Generics { ref params, .. }) |
3837
ItemKind::Enum(_, Generics { ref params, .. }) => {
39-
if attr::contains_name(&annitem.attrs, sym::rustc_copy_clone_marker) &&
38+
let container_id = cx.current_expansion.id.parent();
39+
if cx.resolver.has_derives(container_id, SpecialDerives::COPY) &&
4040
!params.iter().any(|param| match param.kind {
4141
ast::GenericParamKind::Type { .. } => true,
4242
_ => false,

0 commit comments

Comments
 (0)