Rollup merge of #63867 - petrochenkov:dhelpers, r=matthewjasper
resolve: Block expansion of a derive container until all its derives are resolved So, it turns out there's one more reason to block expansion of a `#[derive]` container until all the derives inside it are resolved, beside `Copy` (https://github.com/rust-lang/rust/pull/63248). The set of derive helper attributes registered by derives in the container also has to be known before the derives themselves are expanded, otherwise it may be too late (see https://github.com/rust-lang/rust/pull/63468#issuecomment-524550872 and the `#[stable_hasher]`-related test failures in https://github.com/rust-lang/rust/pull/63468). So, we stop our attempts to unblock the container earlier, as soon as the `Copy` status is known, and just block until all its derives are resolved. After all the derives are resolved we immediately go and process their helper attributes in the item, without delaying it until expansion of the individual derives. Unblocks https://github.com/rust-lang/rust/pull/63468 r? @matthewjasper (as a reviewer of https://github.com/rust-lang/rust/pull/63248) cc @c410-f3r
This commit is contained in:
@@ -11,6 +11,7 @@ use crate::ptr::P;
|
||||
use crate::symbol::{kw, sym, Ident, Symbol};
|
||||
use crate::{ThinVec, MACRO_ARGUMENTS};
|
||||
use crate::tokenstream::{self, TokenStream, TokenTree};
|
||||
use crate::visit::Visitor;
|
||||
|
||||
use errors::{DiagnosticBuilder, DiagnosticId};
|
||||
use smallvec::{smallvec, SmallVec};
|
||||
@@ -72,6 +73,17 @@ impl Annotatable {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn visit_with<'a, V: Visitor<'a>>(&'a self, visitor: &mut V) {
|
||||
match self {
|
||||
Annotatable::Item(item) => visitor.visit_item(item),
|
||||
Annotatable::TraitItem(trait_item) => visitor.visit_trait_item(trait_item),
|
||||
Annotatable::ImplItem(impl_item) => visitor.visit_impl_item(impl_item),
|
||||
Annotatable::ForeignItem(foreign_item) => visitor.visit_foreign_item(foreign_item),
|
||||
Annotatable::Stmt(stmt) => visitor.visit_stmt(stmt),
|
||||
Annotatable::Expr(expr) => visitor.visit_expr(expr),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn expect_item(self) -> P<ast::Item> {
|
||||
match self {
|
||||
Annotatable::Item(i) => i,
|
||||
@@ -700,6 +712,12 @@ impl SyntaxExtension {
|
||||
|
||||
pub type NamedSyntaxExtension = (Name, SyntaxExtension);
|
||||
|
||||
/// Result of resolving a macro invocation.
|
||||
pub enum InvocationRes {
|
||||
Single(Lrc<SyntaxExtension>),
|
||||
DeriveContainer(Vec<Lrc<SyntaxExtension>>),
|
||||
}
|
||||
|
||||
/// Error type that denotes indeterminacy.
|
||||
pub struct Indeterminate;
|
||||
|
||||
@@ -727,7 +745,7 @@ pub trait Resolver {
|
||||
|
||||
fn resolve_macro_invocation(
|
||||
&mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool
|
||||
) -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;
|
||||
) -> Result<InvocationRes, Indeterminate>;
|
||||
|
||||
fn check_unused_macros(&self);
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ use crate::attr::{self, HasAttrs};
|
||||
use crate::source_map::respan;
|
||||
use crate::config::StripUnconfigured;
|
||||
use crate::ext::base::*;
|
||||
use crate::ext::proc_macro::collect_derives;
|
||||
use crate::ext::proc_macro::{collect_derives, MarkAttrs};
|
||||
use crate::ext::hygiene::{ExpnId, SyntaxContext, ExpnData, ExpnKind};
|
||||
use crate::ext::tt::macro_rules::annotate_err_with_kind;
|
||||
use crate::ext::placeholders::{placeholder, PlaceholderExpander};
|
||||
@@ -307,10 +307,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
|
||||
|
||||
let eager_expansion_root =
|
||||
if self.monotonic { invoc.expansion_data.id } else { orig_expansion_data.id };
|
||||
let ext = match self.cx.resolver.resolve_macro_invocation(
|
||||
let res = match self.cx.resolver.resolve_macro_invocation(
|
||||
&invoc, eager_expansion_root, force
|
||||
) {
|
||||
Ok(ext) => ext,
|
||||
Ok(res) => res,
|
||||
Err(Indeterminate) => {
|
||||
undetermined_invocations.push(invoc);
|
||||
continue
|
||||
@@ -322,54 +322,72 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
|
||||
self.cx.current_expansion = invoc.expansion_data.clone();
|
||||
|
||||
// FIXME(jseyfried): Refactor out the following logic
|
||||
let (expanded_fragment, new_invocations) = if let Some(ext) = ext {
|
||||
let fragment = self.expand_invoc(invoc, &ext.kind);
|
||||
self.collect_invocations(fragment, &[])
|
||||
} else if let InvocationKind::DeriveContainer { derives: traits, item } = invoc.kind {
|
||||
if !item.derive_allowed() {
|
||||
let attr = attr::find_by_name(item.attrs(), sym::derive)
|
||||
.expect("`derive` attribute should exist");
|
||||
let span = attr.span;
|
||||
let mut err = self.cx.mut_span_err(span,
|
||||
"`derive` may only be applied to \
|
||||
structs, enums and unions");
|
||||
if let ast::AttrStyle::Inner = attr.style {
|
||||
let trait_list = traits.iter()
|
||||
.map(|t| t.to_string()).collect::<Vec<_>>();
|
||||
let suggestion = format!("#[derive({})]", trait_list.join(", "));
|
||||
err.span_suggestion(
|
||||
span, "try an outer attribute", suggestion,
|
||||
// We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT
|
||||
Applicability::MaybeIncorrect
|
||||
);
|
||||
let (expanded_fragment, new_invocations) = match res {
|
||||
InvocationRes::Single(ext) => {
|
||||
let fragment = self.expand_invoc(invoc, &ext.kind);
|
||||
self.collect_invocations(fragment, &[])
|
||||
}
|
||||
InvocationRes::DeriveContainer(exts) => {
|
||||
let (derives, item) = match invoc.kind {
|
||||
InvocationKind::DeriveContainer { derives, item } => (derives, item),
|
||||
_ => unreachable!(),
|
||||
};
|
||||
if !item.derive_allowed() {
|
||||
let attr = attr::find_by_name(item.attrs(), sym::derive)
|
||||
.expect("`derive` attribute should exist");
|
||||
let span = attr.span;
|
||||
let mut err = self.cx.mut_span_err(span,
|
||||
"`derive` may only be applied to structs, enums and unions");
|
||||
if let ast::AttrStyle::Inner = attr.style {
|
||||
let trait_list = derives.iter()
|
||||
.map(|t| t.to_string()).collect::<Vec<_>>();
|
||||
let suggestion = format!("#[derive({})]", trait_list.join(", "));
|
||||
err.span_suggestion(
|
||||
span, "try an outer attribute", suggestion,
|
||||
// We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT
|
||||
Applicability::MaybeIncorrect
|
||||
);
|
||||
}
|
||||
err.emit();
|
||||
}
|
||||
err.emit();
|
||||
}
|
||||
|
||||
let mut item = self.fully_configure(item);
|
||||
item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
|
||||
let derive_placeholders =
|
||||
all_derive_placeholders.entry(invoc.expansion_data.id).or_default();
|
||||
let mut item = self.fully_configure(item);
|
||||
item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
|
||||
let mut helper_attrs = Vec::new();
|
||||
let mut has_copy = false;
|
||||
for ext in exts {
|
||||
helper_attrs.extend(&ext.helper_attrs);
|
||||
has_copy |= ext.is_derive_copy;
|
||||
}
|
||||
// Mark derive helpers inside this item as known and used.
|
||||
// FIXME: This is a hack, derive helpers should be integrated with regular name
|
||||
// resolution instead. For example, helpers introduced by a derive container
|
||||
// can be in scope for all code produced by that container's expansion.
|
||||
item.visit_with(&mut MarkAttrs(&helper_attrs));
|
||||
if has_copy {
|
||||
self.cx.resolver.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
|
||||
}
|
||||
|
||||
derive_placeholders.reserve(traits.len());
|
||||
invocations.reserve(traits.len());
|
||||
for path in traits {
|
||||
let expn_id = ExpnId::fresh(None);
|
||||
derive_placeholders.push(NodeId::placeholder_from_expn_id(expn_id));
|
||||
invocations.push(Invocation {
|
||||
kind: InvocationKind::Derive { path, item: item.clone() },
|
||||
fragment_kind: invoc.fragment_kind,
|
||||
expansion_data: ExpansionData {
|
||||
id: expn_id,
|
||||
..invoc.expansion_data.clone()
|
||||
},
|
||||
});
|
||||
let derive_placeholders =
|
||||
all_derive_placeholders.entry(invoc.expansion_data.id).or_default();
|
||||
derive_placeholders.reserve(derives.len());
|
||||
invocations.reserve(derives.len());
|
||||
for path in derives {
|
||||
let expn_id = ExpnId::fresh(None);
|
||||
derive_placeholders.push(NodeId::placeholder_from_expn_id(expn_id));
|
||||
invocations.push(Invocation {
|
||||
kind: InvocationKind::Derive { path, item: item.clone() },
|
||||
fragment_kind: invoc.fragment_kind,
|
||||
expansion_data: ExpansionData {
|
||||
id: expn_id,
|
||||
..invoc.expansion_data.clone()
|
||||
},
|
||||
});
|
||||
}
|
||||
let fragment = invoc.fragment_kind
|
||||
.expect_from_annotatables(::std::iter::once(item));
|
||||
self.collect_invocations(fragment, derive_placeholders)
|
||||
}
|
||||
let fragment = invoc.fragment_kind
|
||||
.expect_from_annotatables(::std::iter::once(item));
|
||||
self.collect_invocations(fragment, derive_placeholders)
|
||||
} else {
|
||||
unreachable!()
|
||||
};
|
||||
|
||||
if expanded_fragments.len() < depth {
|
||||
|
||||
@@ -78,7 +78,6 @@ pub struct ProcMacroDerive {
|
||||
pub client: proc_macro::bridge::client::Client<
|
||||
fn(proc_macro::TokenStream) -> proc_macro::TokenStream,
|
||||
>,
|
||||
pub attrs: Vec<ast::Name>,
|
||||
}
|
||||
|
||||
impl MultiItemModifier for ProcMacroDerive {
|
||||
@@ -111,9 +110,6 @@ impl MultiItemModifier for ProcMacroDerive {
|
||||
}
|
||||
}
|
||||
|
||||
// Mark attributes as known, and used.
|
||||
MarkAttrs(&self.attrs).visit_item(&item);
|
||||
|
||||
let token = token::Interpolated(Lrc::new(token::NtItem(item)));
|
||||
let input = tokenstream::TokenTree::token(token, DUMMY_SP).into();
|
||||
|
||||
@@ -164,7 +160,7 @@ impl MultiItemModifier for ProcMacroDerive {
|
||||
}
|
||||
}
|
||||
|
||||
struct MarkAttrs<'a>(&'a [ast::Name]);
|
||||
crate struct MarkAttrs<'a>(crate &'a [ast::Name]);
|
||||
|
||||
impl<'a> Visitor<'a> for MarkAttrs<'a> {
|
||||
fn visit_attribute(&mut self, attr: &Attribute) {
|
||||
|
||||
Reference in New Issue
Block a user