Skip to content

Commit

Permalink
Auto merge of rust-lang#79078 - petrochenkov:derattr, r=Aaron1011
Browse files Browse the repository at this point in the history
expand/resolve: Turn `#[derive]` into a regular macro attribute

This PR turns `#[derive]` into a regular attribute macro declared in libcore and defined in `rustc_builtin_macros`, like it was previously done with other "active" attributes in rust-lang#62086, rust-lang#62735 and other PRs.
This PR is also a continuation of rust-lang#65252, rust-lang#69870 and other PRs linked from them, which layed the ground for converting `#[derive]` specifically.

`#[derive]` still asks `rustc_resolve` to resolve paths inside `derive(...)`, and `rustc_expand` gets those resolution results through some backdoor (which I'll try to address later), but otherwise `#[derive]` is treated as any other macro attributes, which simplifies the resolution-expansion infra pretty significantly.

The change has several observable effects on language and library.
Some of the language changes are **feature-gated** by [`feature(macro_attributes_in_derive_output)`](rust-lang#81119).

#### Library

- `derive` is now available through standard library as `{core,std}::prelude::v1::derive`.

#### Language

- `derive` now goes through name resolution, so it can now be renamed - `use derive as my_derive; #[my_derive(Debug)] struct S;`.
- `derive` now goes through name resolution, so this resolution can fail in corner cases. Crater found one such regression, where import `use foo as derive` goes into a cycle with `#[derive(Something)]`.
- **[feature-gated]** `#[derive]` is now expanded as any other attributes in left-to-right order. This allows to remove the restriction on other macro attributes following `#[derive]` (rust-lang/reference#566). The following macro attributes become a part of the derive's input (this is not a change, non-macro attributes following `#[derive]` were treated in the same way previously).
- `#[derive]` is now expanded as any other attributes in left-to-right order. This means two derive attributes `#[derive(Foo)] #[derive(Bar)]` are now expanded separately rather than together. It doesn't generally make difference, except for esoteric cases. For example `#[derive(Foo)]` can now produce an import bringing `Bar` into scope, but previously both `Foo` and `Bar` were required to be resolved before expanding any of them.
- **[feature-gated]** `#[derive()]` (with empty list in parentheses) actually becomes useful. For historical reasons `#[derive]` *fully configures* its input, eagerly evaluating `cfg` everywhere in its target, for example on fields.
Expansion infra doesn't do that for other attributes, but now when macro attributes attributes are allowed to be written after `#[derive]`, it means that derive can *fully configure* items for them.
    ```rust
	#[derive()]
	#[my_attr]
	struct S {
		#[cfg(FALSE)] // this field in removed by `#[derive()]` and not observed by `#[my_attr]`
		field: u8
	}
    ```
- `#[derive]` on some non-item targets is now prohibited. This was accidentally allowed as noop in the past, but was warned about since early 2018 (rust-lang#50092), despite that crater found a few such cases in unmaintained crates.
- Derive helper attributes used before their introduction are now reported with a deprecation lint. This change is long overdue (since macro modularization, rust-lang#52226 (comment)), but it was hard to do without fixing expansion order for derives. The deprecation is tracked by rust-lang#79202.
```rust
    #[trait_helper] // warning: derive helper attribute is used before it is introduced
    #[derive(Trait)]
    struct S {}
```

Crater analysis: rust-lang#79078 (comment)
  • Loading branch information
bors committed Feb 7, 2021
2 parents 36ecbc9 + d8af6de commit 9778068
Show file tree
Hide file tree
Showing 65 changed files with 1,396 additions and 1,008 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3582,6 +3582,7 @@ dependencies = [
"rustc_errors",
"rustc_expand",
"rustc_feature",
"rustc_lexer",
"rustc_parse",
"rustc_parse_format",
"rustc_session",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ rustc_attr = { path = "../rustc_attr" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_feature = { path = "../rustc_feature" }
rustc_lexer = { path = "../rustc_lexer" }
rustc_parse = { path = "../rustc_parse" }
rustc_target = { path = "../rustc_target" }
rustc_session = { path = "../rustc_session" }
Expand Down
132 changes: 132 additions & 0 deletions compiler/rustc_builtin_macros/src/derive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
use rustc_ast::{self as ast, token, ItemKind, MetaItemKind, NestedMetaItem, StmtKind};
use rustc_errors::{struct_span_err, Applicability};
use rustc_expand::base::{Annotatable, ExpandResult, ExtCtxt, Indeterminate, MultiItemModifier};
use rustc_expand::config::StripUnconfigured;
use rustc_feature::AttributeTemplate;
use rustc_parse::validate_attr;
use rustc_session::Session;
use rustc_span::symbol::sym;
use rustc_span::Span;

crate struct Expander;

impl MultiItemModifier for Expander {
fn expand(
&self,
ecx: &mut ExtCtxt<'_>,
span: Span,
meta_item: &ast::MetaItem,
item: Annotatable,
) -> ExpandResult<Vec<Annotatable>, Annotatable> {
let sess = ecx.sess;
if report_bad_target(sess, &item, span) {
// We don't want to pass inappropriate targets to derive macros to avoid
// follow up errors, all other errors below are recoverable.
return ExpandResult::Ready(vec![item]);
}

let template =
AttributeTemplate { list: Some("Trait1, Trait2, ..."), ..Default::default() };
let attr = ecx.attribute(meta_item.clone());
validate_attr::check_builtin_attribute(&sess.parse_sess, &attr, sym::derive, template);

let derives: Vec<_> = attr
.meta_item_list()
.unwrap_or_default()
.into_iter()
.filter_map(|nested_meta| match nested_meta {
NestedMetaItem::MetaItem(meta) => Some(meta),
NestedMetaItem::Literal(lit) => {
// Reject `#[derive("Debug")]`.
report_unexpected_literal(sess, &lit);
None
}
})
.map(|meta| {
// Reject `#[derive(Debug = "value", Debug(abc))]`, but recover the paths.
report_path_args(sess, &meta);
meta.path
})
.collect();

// FIXME: Try to cache intermediate results to avoid collecting same paths multiple times.
match ecx.resolver.resolve_derives(ecx.current_expansion.id, derives, ecx.force_mode) {
Ok(()) => {
let mut visitor =
StripUnconfigured { sess, features: ecx.ecfg.features, modified: false };
let mut item = visitor.fully_configure(item);
if visitor.modified {
// Erase the tokens if cfg-stripping modified the item
// This will cause us to synthesize fake tokens
// when `nt_to_tokenstream` is called on this item.
match &mut item {
Annotatable::Item(item) => item,
Annotatable::Stmt(stmt) => match &mut stmt.kind {
StmtKind::Item(item) => item,
_ => unreachable!(),
},
_ => unreachable!(),
}
.tokens = None;
}
ExpandResult::Ready(vec![item])
}
Err(Indeterminate) => ExpandResult::Retry(item),
}
}
}

fn report_bad_target(sess: &Session, item: &Annotatable, span: Span) -> bool {
let item_kind = match item {
Annotatable::Item(item) => Some(&item.kind),
Annotatable::Stmt(stmt) => match &stmt.kind {
StmtKind::Item(item) => Some(&item.kind),
_ => None,
},
_ => None,
};

let bad_target =
!matches!(item_kind, Some(ItemKind::Struct(..) | ItemKind::Enum(..) | ItemKind::Union(..)));
if bad_target {
struct_span_err!(
sess,
span,
E0774,
"`derive` may only be applied to structs, enums and unions",
)
.emit();
}
bad_target
}

fn report_unexpected_literal(sess: &Session, lit: &ast::Lit) {
let help_msg = match lit.token.kind {
token::Str if rustc_lexer::is_ident(&lit.token.symbol.as_str()) => {
format!("try using `#[derive({})]`", lit.token.symbol)
}
_ => "for example, write `#[derive(Debug)]` for `Debug`".to_string(),
};
struct_span_err!(sess, lit.span, E0777, "expected path to a trait, found literal",)
.help(&help_msg)
.emit();
}

fn report_path_args(sess: &Session, meta: &ast::MetaItem) {
let report_error = |title, action| {
let span = meta.span.with_lo(meta.path.span.hi());
sess.struct_span_err(span, title)
.span_suggestion(span, action, String::new(), Applicability::MachineApplicable)
.emit();
};
match meta.kind {
MetaItemKind::Word => {}
MetaItemKind::List(..) => report_error(
"traits in `#[derive(...)]` don't accept arguments",
"remove the arguments",
),
MetaItemKind::NameValue(..) => {
report_error("traits in `#[derive(...)]` don't accept values", "remove the value")
}
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_builtin_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ mod cfg_accessible;
mod compile_error;
mod concat;
mod concat_idents;
mod derive;
mod deriving;
mod env;
mod format;
Expand Down Expand Up @@ -88,6 +89,7 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
register_attr! {
bench: test::expand_bench,
cfg_accessible: cfg_accessible::Expander,
derive: derive::Expander,
global_allocator: global_allocator::expand,
test: test::expand_test,
test_case: test::expand_test_case,
Expand Down
47 changes: 19 additions & 28 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ impl Annotatable {
}

crate fn into_tokens(self, sess: &ParseSess) -> TokenStream {
nt_to_tokenstream(&self.into_nonterminal(), sess, CanSynthesizeMissingTokens::No)
// Tokens of an attribute target may be invalidated by some outer `#[derive]` performing
// "full configuration" (attributes following derives on the same item should be the most
// common case), that's why synthesizing tokens is allowed.
nt_to_tokenstream(&self.into_nonterminal(), sess, CanSynthesizeMissingTokens::Yes)
}

pub fn expect_item(self) -> P<ast::Item> {
Expand Down Expand Up @@ -234,25 +237,6 @@ impl Annotatable {
_ => panic!("expected variant"),
}
}

pub fn derive_allowed(&self) -> bool {
match *self {
Annotatable::Stmt(ref stmt) => match stmt.kind {
ast::StmtKind::Item(ref item) => matches!(
item.kind,
ast::ItemKind::Struct(..) | ast::ItemKind::Enum(..) | ast::ItemKind::Union(..)
),
_ => false,
},
Annotatable::Item(ref item) => match item.kind {
ast::ItemKind::Struct(..) | ast::ItemKind::Enum(..) | ast::ItemKind::Union(..) => {
true
}
_ => false,
},
_ => false,
}
}
}

/// Result of an expansion that may need to be retried.
Expand Down Expand Up @@ -854,12 +838,6 @@ impl 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;

Expand All @@ -885,16 +863,29 @@ pub trait ResolverExpand {
invoc: &Invocation,
eager_expansion_root: ExpnId,
force: bool,
) -> Result<InvocationRes, Indeterminate>;
) -> Result<Lrc<SyntaxExtension>, Indeterminate>;

fn check_unused_macros(&mut self);

/// Some parent node that is close enough to the given macro call.
fn lint_node_id(&mut self, expn_id: ExpnId) -> NodeId;
fn lint_node_id(&self, expn_id: ExpnId) -> NodeId;

// Resolver interfaces for specific built-in macros.
/// Does `#[derive(...)]` attribute with the given `ExpnId` have built-in `Copy` inside it?
fn has_derive_copy(&self, expn_id: ExpnId) -> bool;
/// Resolve paths inside the `#[derive(...)]` attribute with the given `ExpnId`.
fn resolve_derives(
&mut self,
expn_id: ExpnId,
derives: Vec<ast::Path>,
force: bool,
) -> Result<(), Indeterminate>;
/// Take resolutions for paths inside the `#[derive(...)]` attribute with the given `ExpnId`
/// back from resolver.
fn take_derive_resolutions(
&mut self,
expn_id: ExpnId,
) -> Option<Vec<(Lrc<SyntaxExtension>, ast::Path)>>;
/// Path resolution logic for `#[cfg_accessible(path)]`.
fn cfg_accessible(&mut self, expn_id: ExpnId, path: &ast::Path) -> Result<bool, Indeterminate>;
}
Expand Down
Loading

0 comments on commit 9778068

Please sign in to comment.