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

Move special treatment of derive(Copy, PartialEq, Eq) from expansion infrastructure to elsewhere #63248

Merged
merged 1 commit into from
Aug 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use syntax::ast;
use syntax::ptr::P as AstP;
use syntax::ast::*;
use syntax::errors;
use syntax::ext::base::SpecialDerives;
use syntax::ext::hygiene::ExpnId;
use syntax::print::pprust;
use syntax::source_map::{respan, ExpnInfo, ExpnKind, DesugaringKind, Spanned};
Expand Down Expand Up @@ -176,6 +177,8 @@ pub trait Resolver {
components: &[Symbol],
is_value: bool,
) -> (ast::Path, Res<NodeId>);

fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool;
}

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

fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
fn lower_attrs_extendable(&mut self, attrs: &[Attribute]) -> Vec<Attribute> {
attrs
.iter()
.map(|a| self.lower_attr(a))
.collect()
}

fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
self.lower_attrs_extendable(attrs).into()
}

fn lower_attr(&mut self, attr: &Attribute) -> Attribute {
// Note that we explicitly do not walk the path. Since we don't really
// lower attributes (we use the AST version) there is nowhere to keep
Expand Down Expand Up @@ -4028,7 +4035,14 @@ impl<'a> LoweringContext<'a> {
pub fn lower_item(&mut self, i: &Item) -> Option<hir::Item> {
let mut ident = i.ident;
let mut vis = self.lower_visibility(&i.vis, None);
let attrs = self.lower_attrs(&i.attrs);
let mut attrs = self.lower_attrs_extendable(&i.attrs);
if self.resolver.has_derives(i.id, SpecialDerives::PARTIAL_EQ | SpecialDerives::EQ) {
// Add `#[structural_match]` if the item derived both `PartialEq` and `Eq`.
let ident = Ident::new(sym::structural_match, i.span);
attrs.push(attr::mk_attr_outer(attr::mk_word_item(ident)));
}
let attrs = attrs.into();

if let ItemKind::MacroDef(ref def) = i.node {
if !def.legacy || attr::contains_name(&i.attrs, sym::macro_export) {
let body = self.lower_token_stream(def.stream());
Expand Down
20 changes: 18 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ use rustc_metadata::cstore::CStore;
use syntax::source_map::SourceMap;
use syntax::ext::hygiene::{ExpnId, Transparency, SyntaxContext};
use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
use syntax::ext::base::SyntaxExtension;
use syntax::ext::base::MacroKind;
use syntax::ext::base::{SyntaxExtension, MacroKind, SpecialDerives};
use syntax::symbol::{Symbol, kw, sym};
use syntax::util::lev_distance::find_best_match_for_name;

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

Choose a reason for hiding this comment

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

I would think the better way to do this would be a perma unstable trait:

#[lang = "structural_match"]
unsafe trait Structural {}

for which implementations are generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, something like that would also work.
PartialEq produces impl Structural, rustc_mir somehow checks for T: Structural + Eq.

Copy link
Contributor

@Centril Centril Aug 3, 2019

Choose a reason for hiding this comment

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

cc also @varkor

This could be useful for const generics (#60286), e.g. fn foo<T: Structural, const A: T>();. (But that would require #[derive(PartialEq, Eq)] -> Structural, not just PartialEq.)

special_derives: FxHashMap<ExpnId, SpecialDerives>,

/// Maps the `ExpnId` of an expansion to its containing module or block.
invocations: FxHashMap<ExpnId, &'a InvocationData<'a>>,
Expand Down Expand Up @@ -1813,6 +1818,12 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
fn definitions(&mut self) -> &mut Definitions {
&mut self.definitions
}

fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool {
let def_id = self.definitions.local_def_id(node_id);
let expn_id = self.definitions.expansion_that_defined(def_id.index);
self.has_derives(expn_id, derives)
}
}

impl<'a> Resolver<'a> {
Expand Down Expand Up @@ -2031,6 +2042,7 @@ impl<'a> Resolver<'a> {
struct_constructors: Default::default(),
unused_macros: Default::default(),
proc_macro_stubs: Default::default(),
special_derives: Default::default(),
current_type_ascription: Vec::new(),
injected_crate: None,
active_features:
Expand Down Expand Up @@ -2077,6 +2089,10 @@ impl<'a> Resolver<'a> {
}
}

fn has_derives(&self, expn_id: ExpnId, markers: SpecialDerives) -> bool {
self.special_derives.get(&expn_id).map_or(false, |m| m.contains(markers))
}

/// Entry point to crate resolution.
pub fn resolve_crate(&mut self, krate: &Crate) {
ImportResolver { resolver: self }.finalize_imports();
Expand Down
34 changes: 31 additions & 3 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc::{ty, lint, span_bug};
use syntax::ast::{self, Ident, ItemKind};
use syntax::attr::{self, StabilityLevel};
use syntax::edition::Edition;
use syntax::ext::base::{self, Indeterminate};
use syntax::ext::base::{self, Indeterminate, SpecialDerives};
use syntax::ext::base::{MacroKind, SyntaxExtension};
use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
use syntax::ext::hygiene::{self, ExpnId, ExpnInfo, ExpnKind};
Expand Down Expand Up @@ -203,8 +203,28 @@ impl<'a> base::Resolver for Resolver<'a> {
(&mac.node.path, MacroKind::Bang, Vec::new(), false),
InvocationKind::Derive { ref path, .. } =>
(path, MacroKind::Derive, Vec::new(), false),
InvocationKind::DeriveContainer { .. } =>
return Ok(None),
InvocationKind::DeriveContainer { ref derives, .. } => {
// Block expansion of derives in the container until we know whether one of them
// is a built-in `Copy`. Skip the resolution if there's only one derive - either
// it's not a `Copy` and we don't need to do anything, or it's a `Copy` and it
// will automatically knows about itself.
let mut result = Ok(None);
if derives.len() > 1 {
let parent_scope = self.invoc_parent_scope(invoc_id, Vec::new());
for path in derives {
match self.resolve_macro_path(path, Some(MacroKind::Derive),
&parent_scope, true, force) {
Ok((Some(ref ext), _)) if ext.is_derive_copy => {
self.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
return Ok(None);
}
Err(Determinacy::Undetermined) => result = Err(Indeterminate),
_ => {}
}
}
}
return result;
}
};

let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
Expand Down Expand Up @@ -234,6 +254,14 @@ impl<'a> base::Resolver for Resolver<'a> {
);
}
}

fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool {
self.has_derives(expn_id, derives)
}

fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives) {
*self.special_derives.entry(expn_id).or_default() |= derives;
}
}

impl<'a> Resolver<'a> {
Expand Down
18 changes: 16 additions & 2 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,8 @@ pub struct SyntaxExtension {
/// Built-in macros have a couple of special properties (meaning of `$crate`,
/// availability in `#[no_implicit_prelude]` modules), so we have to keep this flag.
pub is_builtin: bool,
/// We have to identify macros providing a `Copy` impl early for compatibility reasons.
pub is_derive_copy: bool,
}

impl SyntaxExtensionKind {
Expand Down Expand Up @@ -640,6 +642,7 @@ impl SyntaxExtension {
helper_attrs: Vec::new(),
edition,
is_builtin: false,
is_derive_copy: false,
kind,
}
}
Expand Down Expand Up @@ -683,6 +686,16 @@ pub type NamedSyntaxExtension = (Name, SyntaxExtension);
/// Error type that denotes indeterminacy.
pub struct Indeterminate;

bitflags::bitflags! {
/// Built-in derives that need some extra tracking beyond the usual macro functionality.
#[derive(Default)]
pub struct SpecialDerives: u8 {
const PARTIAL_EQ = 1 << 0;
const EQ = 1 << 1;
const COPY = 1 << 2;
}
}

pub trait Resolver {
fn next_node_id(&mut self) -> ast::NodeId;

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

fn check_unused_macros(&self);

fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool;
fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives);
}

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

impl<'a> ExtCtxt<'a> {
Expand All @@ -745,7 +760,6 @@ impl<'a> ExtCtxt<'a> {
directory_ownership: DirectoryOwnership::Owned { relative: None },
},
expansions: FxHashMap::default(),
allow_derive_markers: [sym::rustc_attrs, sym::structural_match][..].into(),
}
}

Expand Down
19 changes: 4 additions & 15 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::attr::{self, HasAttrs};
use crate::source_map::{dummy_spanned, respan};
use crate::config::StripUnconfigured;
use crate::ext::base::*;
use crate::ext::proc_macro::{add_derived_markers, collect_derives};
use crate::ext::proc_macro::collect_derives;
use crate::ext::hygiene::{ExpnId, SyntaxContext, ExpnInfo, ExpnKind};
use crate::ext::placeholders::{placeholder, PlaceholderExpander};
use crate::feature_gate::{self, Features, GateIssue, is_builtin_attr, emit_feature_err};
Expand Down Expand Up @@ -209,7 +209,6 @@ pub enum InvocationKind {
Derive {
path: Path,
item: Annotatable,
item_with_markers: Annotatable,
},
/// "Invocation" that contains all derives from an item,
/// broken into multiple `Derive` invocations when expanded.
Expand Down Expand Up @@ -360,8 +359,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {

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

derives.reserve(traits.len());
Expand All @@ -370,11 +367,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let expn_id = ExpnId::fresh(self.cx.current_expansion.id, None);
derives.push(expn_id);
invocations.push(Invocation {
kind: InvocationKind::Derive {
path,
item: item.clone(),
item_with_markers: item_with_markers.clone(),
},
kind: InvocationKind::Derive { path, item: item.clone() },
fragment_kind: invoc.fragment_kind,
expansion_data: ExpansionData {
id: expn_id,
Expand All @@ -383,7 +376,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
});
}
let fragment = invoc.fragment_kind
.expect_from_annotatables(::std::iter::once(item_with_markers));
.expect_from_annotatables(::std::iter::once(item));
self.collect_invocations(fragment, derives)
} else {
unreachable!()
Expand Down Expand Up @@ -569,13 +562,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
_ => unreachable!()
}
InvocationKind::Derive { path, item, item_with_markers } => match ext {
InvocationKind::Derive { path, item } => match ext {
SyntaxExtensionKind::Derive(expander) |
SyntaxExtensionKind::LegacyDerive(expander) => {
let (path, item) = match ext {
SyntaxExtensionKind::LegacyDerive(..) => (path, item_with_markers),
_ => (path, item),
};
if !item.derive_allowed() {
return fragment_kind.dummy(span);
}
Expand Down
35 changes: 2 additions & 33 deletions src/libsyntax/ext/proc_macro.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use crate::ast::{self, ItemKind, Attribute, Mac};
use crate::attr::{mark_used, mark_known, HasAttrs};
use crate::attr::{mark_used, mark_known};
use crate::errors::{Applicability, FatalError};
use crate::ext::base::{self, *};
use crate::ext::proc_macro_server;
use crate::parse::{self, token};
use crate::parse::parser::PathStyle;
use crate::symbol::{sym, Symbol};
use crate::symbol::sym;
use crate::tokenstream::{self, TokenStream};
use crate::visit::Visitor;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::sync::Lrc;
use syntax_pos::hygiene::{ExpnInfo, ExpnKind};
use syntax_pos::{Span, DUMMY_SP};

const EXEC_STRATEGY: proc_macro::bridge::server::SameThread =
Expand Down Expand Up @@ -217,32 +215,3 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>)
});
result
}

crate fn add_derived_markers<T: HasAttrs>(
cx: &mut ExtCtxt<'_>, span: Span, traits: &[ast::Path], item: &mut T
) {
let (mut names, mut pretty_name) = (FxHashSet::default(), String::new());
for (i, path) in traits.iter().enumerate() {
if i > 0 {
pretty_name.push_str(", ");
}
pretty_name.push_str(&path.to_string());
names.insert(unwrap_or!(path.segments.get(0), continue).ident.name);
}

let span = span.fresh_expansion(cx.current_expansion.id, ExpnInfo::allow_unstable(
ExpnKind::Macro(MacroKind::Derive, Symbol::intern(&pretty_name)), span,
cx.parse_sess.edition, cx.allow_derive_markers.clone(),
));

item.visit_attrs(|attrs| {
if names.contains(&sym::Eq) && names.contains(&sym::PartialEq) {
let meta = cx.meta_word(span, sym::structural_match);
attrs.push(cx.attribute(meta));
}
if names.contains(&sym::Copy) {
let meta = cx.meta_word(span, sym::rustc_copy_clone_marker);
attrs.push(cx.attribute(meta));
}
});
}
5 changes: 4 additions & 1 deletion src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,8 @@ pub fn compile(
}
}

let is_builtin = attr::contains_name(&def.attrs, sym::rustc_builtin_macro);

SyntaxExtension {
kind: SyntaxExtensionKind::LegacyBang(expander),
span: def.span,
Expand All @@ -440,7 +442,8 @@ pub fn compile(
deprecation: attr::find_deprecation(&sess, &def.attrs, def.span),
helper_attrs: Vec::new(),
edition,
is_builtin: attr::contains_name(&def.attrs, sym::rustc_builtin_macro),
is_builtin,
is_derive_copy: is_builtin && def.ident.name == sym::Copy,
}
}

Expand Down
5 changes: 0 additions & 5 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,11 +1329,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
/*opt*/ attributes(name1, name2, ...)"),
Ungated),

(sym::rustc_copy_clone_marker, Whitelisted, template!(Word), Gated(Stability::Unstable,
sym::rustc_attrs,
"internal implementation detail",
cfg_fn!(rustc_attrs))),

(sym::rustc_allocator, Whitelisted, template!(Word), Gated(Stability::Unstable,
sym::rustc_attrs,
"internal implementation detail",
Expand Down
6 changes: 3 additions & 3 deletions src/libsyntax_ext/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use crate::deriving::generic::*;
use crate::deriving::generic::ty::*;

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