Skip to content

Commit

Permalink
Auto merge of #121053 - clubby789:derive-skip, r=<try>
Browse files Browse the repository at this point in the history
Implement `#[skip]` for builtin derives

Implement #121050

Still needs some work but here's an initial working implementation to get feedback on the strategy.
  • Loading branch information
bors committed Apr 24, 2024
2 parents c1feb3e + 93300c6 commit 0d3c0c4
Show file tree
Hide file tree
Showing 22 changed files with 355 additions and 24 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ builtin_macros_derive_path_args_list = traits in `#[derive(...)]` don't accept a
builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept values
.suggestion = remove the value
builtin_macros_derive_skip_bad_argument = incorrect usage of the `#[skip]` attribute
.note = the `#[skip]` attribute accepts an optional list of traits
.help = try using `#[skip]` or `#[skip(Trait)]`
builtin_macros_derive_skip_unsupported = the `#[skip]` attribute does not support this trait
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
.custom = use `std::env::var({$var_expr})` to read the variable at run time
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ pub fn expand_deriving_eq(
cx.attr_nested_word(sym::coverage, sym::off, span)
],
fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
cs_total_eq_assert(a, b, c)
})),
combine_substructure: combine_substructure(Box::new(cs_total_eq_assert)),
}],
associated_types: Vec::new(),
is_const,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub fn expand_deriving_ord(
ret_ty: Path(path_std!(cmp::Ordering)),
attributes: thin_vec![cx.attr_word(sym::inline, span)],
fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
combine_substructure: combine_substructure(Box::new(|a, b, c| cs_cmp(a, b, c))),
combine_substructure: combine_substructure(Box::new(cs_cmp)),
}],
associated_types: Vec::new(),
is_const,
Expand Down Expand Up @@ -58,6 +58,7 @@ pub fn cs_cmp(cx: &ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockO
cx,
span,
substr,
sym::Ord,
|cx, fold| match fold {
CsFold::Single(field) => {
let [other_expr] = &field.other_selflike_exprs[..] else {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub fn expand_deriving_partial_eq(
cx,
span,
substr,
sym::PartialEq,
|cx, fold| match fold {
CsFold::Single(field) => {
let [other_expr] = &field.other_selflike_exprs[..] else {
Expand Down Expand Up @@ -98,7 +99,7 @@ pub fn expand_deriving_partial_eq(
ret_ty: Path(path_local!(bool)),
attributes: thin_vec![cx.attr_word(sym::inline, span)],
fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
combine_substructure: combine_substructure(Box::new(|a, b, c| cs_eq(a, b, c))),
combine_substructure: combine_substructure(Box::new(cs_eq)),
}];

let trait_def = TraitDef {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ fn cs_partial_cmp(
cx,
span,
substr,
sym::PartialOrd,
|cx, fold| match fold {
CsFold::Single(field) => {
let [other_expr] = &field.other_selflike_exprs[..] else {
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ pub fn expand_deriving_debug(
attributes: thin_vec![cx.attr_word(sym::inline, span)],
fieldless_variants_strategy:
FieldlessVariantsStrategy::SpecializeIfAllVariantsFieldless,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
show_substructure(a, b, c)
})),
combine_substructure: combine_substructure(Box::new(show_substructure)),
}],
associated_types: Vec::new(),
is_const,
Expand Down Expand Up @@ -90,6 +88,10 @@ fn show_substructure(cx: &ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) ->
cx.expr_addr_of(field.span, field.self_expr.clone())
}
}
let fields = fields
.iter()
.filter(|fi| !fi.skipped_derives.is_skipped(sym::Debug))
.collect::<ThinVec<_>>();

if fields.is_empty() {
// Special case for no fields.
Expand Down
130 changes: 124 additions & 6 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_session::lint::builtin::BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{Span, DUMMY_SP};
use smallvec::SmallVec;
use std::cell::RefCell;
use std::iter;
use std::ops::Not;
Expand Down Expand Up @@ -263,6 +264,7 @@ pub enum FieldlessVariantsStrategy {
}

/// All the data about the data structure/method being derived upon.
#[derive(Debug)]
pub struct Substructure<'a> {
/// ident of self
pub type_ident: Ident,
Expand All @@ -273,6 +275,7 @@ pub struct Substructure<'a> {
}

/// Summary of the relevant parts of a struct/enum field.
#[derive(Debug)]
pub struct FieldInfo {
pub span: Span,
/// None for tuple structs/normal enum variants, Some for normal
Expand All @@ -284,15 +287,47 @@ pub struct FieldInfo {
/// The expressions corresponding to references to this field in
/// the other selflike arguments.
pub other_selflike_exprs: Vec<P<Expr>>,
/// The derives for which this field should be ignored
pub skipped_derives: SkippedDerives,
}

#[derive(Copy, Clone)]
/// Derives for which this field should be ignored
#[derive(Debug)]
pub enum SkippedDerives {
/// No `#[skip]`
None,
/// `#[skip(Trait, Names)]`
List(SmallVec<[Symbol; 1]>),
/// `#[skip]` with no arguments
All,
}

impl SkippedDerives {
pub fn add(&mut self, derive: Symbol) {
match self {
Self::None => *self = Self::List(SmallVec::from([derive])),
Self::List(idents) => idents.push(derive),
Self::All => (),
}
}

pub fn is_skipped(&self, derive: Symbol) -> bool {
match self {
Self::None => false,
Self::List(idents) => idents.contains(&derive),
Self::All => true,
}
}
}

#[derive(Copy, Clone, Debug)]
pub enum IsTuple {
No,
Yes,
}

/// Fields for a static method
#[derive(Debug)]
pub enum StaticFields {
/// Tuple and unit structs/enum variants like this.
Unnamed(Vec<Span>, IsTuple),
Expand All @@ -301,6 +336,7 @@ pub enum StaticFields {
}

/// A summary of the possible sets of fields.
#[derive(Debug)]
pub enum SubstructureFields<'a> {
/// A non-static method where `Self` is a struct.
Struct(&'a ast::VariantData, Vec<FieldInfo>),
Expand Down Expand Up @@ -1215,7 +1251,13 @@ impl<'a> MethodDef<'a> {

let self_expr = discr_exprs.remove(0);
let other_selflike_exprs = discr_exprs;
let discr_field = FieldInfo { span, name: None, self_expr, other_selflike_exprs };
let discr_field = FieldInfo {
span,
name: None,
self_expr,
other_selflike_exprs,
skipped_derives: SkippedDerives::None,
};

let discr_let_stmts: ThinVec<_> = iter::zip(&discr_idents, &selflike_args)
.map(|(&ident, selflike_arg)| {
Expand Down Expand Up @@ -1518,7 +1560,12 @@ impl<'a> TraitDef<'a> {
.collect()
}

fn create_fields<F>(&self, struct_def: &'a VariantData, mk_exprs: F) -> Vec<FieldInfo>
fn create_fields<F>(
&self,
cx: &ExtCtxt<'_>,
struct_def: &'a VariantData,
mk_exprs: F,
) -> Vec<FieldInfo>
where
F: Fn(usize, &ast::FieldDef, Span) -> Vec<P<ast::Expr>>,
{
Expand All @@ -1533,11 +1580,76 @@ impl<'a> TraitDef<'a> {
let mut exprs: Vec<_> = mk_exprs(i, struct_field, sp);
let self_expr = exprs.remove(0);
let other_selflike_exprs = exprs;
let mut skipped_derives = SkippedDerives::None;
let skip_enabled = cx.ecfg.features.derive_skip
|| struct_field.span.allows_unstable(sym::derive_skip);
for attr in attr::filter_by_name(&struct_field.attrs, sym::skip) {
if !skip_enabled {
rustc_session::parse::feature_err(
&cx.sess,
sym::derive_skip,
attr.span,
"the `#[skip]` attribute is experimental",
)
.emit();
}
let Some(skip_attr) = ast::Attribute::meta_kind(attr) else {
unreachable!()
};

// FIXME: better errors
match skip_attr {
ast::MetaItemKind::Word => {
skipped_derives = SkippedDerives::All;
break;
}
ast::MetaItemKind::List(items) => {
for item in items {
let span = item.span();
let ast::NestedMetaItem::MetaItem(ast::MetaItem {
path,
kind: ast::MetaItemKind::Word,
..
}) = item
else {
cx.dcx().emit_err(errors::DeriveSkipBadArgument {
span,
});
continue;
};
let name = path.segments[0].ident;
const SUPPORTED_TRAITS: [Symbol; 5] = [
sym::PartialEq,
sym::PartialOrd,
sym::Ord,
sym::Hash,
sym::Debug,
];
if SUPPORTED_TRAITS.contains(&name.name) {
skipped_derives.add(path.segments[0].ident.name)
} else {
let traits = SUPPORTED_TRAITS.iter().map(|s| format!("`{s}`")).collect::<Vec<_>>().join(", ");
cx.psess().buffer_lint_with_diagnostic(
rustc_session::lint::builtin::UNSUPPORTED_DERIVE_SKIP,
span,
cx.current_expansion.lint_node_id,
crate::fluent_generated::builtin_macros_derive_skip_unsupported,
rustc_session::lint::BuiltinLintDiag::DeriveSkipUnsupported { traits },
)
}
}
}
ast::MetaItemKind::NameValue(lit) => {
cx.dcx().emit_err(errors::DeriveSkipBadArgument { span: lit.span });
}
}
}
FieldInfo {
span: sp.with_ctxt(self.span.ctxt()),
name: struct_field.ident,
self_expr,
other_selflike_exprs,
skipped_derives,
}
})
.collect()
Expand All @@ -1553,7 +1665,7 @@ impl<'a> TraitDef<'a> {
struct_def: &'a VariantData,
prefixes: &[String],
) -> Vec<FieldInfo> {
self.create_fields(struct_def, |i, _struct_field, sp| {
self.create_fields(cx, struct_def, |i, _struct_field, sp| {
prefixes
.iter()
.map(|prefix| {
Expand All @@ -1571,7 +1683,7 @@ impl<'a> TraitDef<'a> {
struct_def: &'a VariantData,
is_packed: bool,
) -> Vec<FieldInfo> {
self.create_fields(struct_def, |i, struct_field, sp| {
self.create_fields(cx, struct_def, |i, struct_field, sp| {
selflike_args
.iter()
.map(|selflike_arg| {
Expand Down Expand Up @@ -1667,13 +1779,19 @@ pub fn cs_fold<F>(
cx: &ExtCtxt<'_>,
trait_span: Span,
substructure: &Substructure<'_>,
trait_name: Symbol,
mut f: F,
) -> P<Expr>
where
F: FnMut(&ExtCtxt<'_>, CsFold<'_>) -> P<Expr>,
{
match substructure.fields {
EnumMatching(.., all_fields) | Struct(_, all_fields) => {
let all_fields = all_fields
.iter()
.filter(|fi| !fi.skipped_derives.is_skipped(trait_name))
.collect::<Vec<&FieldInfo>>();

if all_fields.is_empty() {
return f(cx, CsFold::Fieldless);
}
Expand All @@ -1686,7 +1804,7 @@ where

let base_expr = f(cx, CsFold::Single(base_field));

let op = |old, field: &FieldInfo| {
let op = |old, field: &&FieldInfo| {
let new = f(cx, CsFold::Single(field));
f(cx, CsFold::Combine(field.span, old, new))
};
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_builtin_macros/src/deriving/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ pub fn expand_deriving_hash(
ret_ty: Unit,
attributes: thin_vec![cx.attr_word(sym::inline, span)],
fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
hash_substructure(a, b, c)
})),
combine_substructure: combine_substructure(Box::new(hash_substructure)),
}],
associated_types: Vec::new(),
is_const,
Expand All @@ -62,8 +60,11 @@ fn hash_substructure(cx: &ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'

let (stmts, match_expr) = match substr.fields {
Struct(_, fields) | EnumMatching(.., fields) => {
let stmts =
fields.iter().map(|field| call_hash(field.span, field.self_expr.clone())).collect();
let stmts = fields
.iter()
.filter(|fi| !fi.skipped_derives.is_skipped(sym::Hash))
.map(|field| call_hash(field.span, field.self_expr.clone()))
.collect();
(stmts, None)
}
EnumDiscr(discr_field, match_expr) => {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,15 @@ pub(crate) struct DerivePathArgsValue {
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_derive_skip_bad_argument)]
pub(crate) struct DeriveSkipBadArgument {
#[note]
#[help]
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_no_default_variant)]
#[help]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,8 @@ declare_features! (
(unstable, deprecated_suggestion, "1.61.0", Some(94785)),
/// Allows deref patterns.
(incomplete, deref_patterns, "CURRENT_RUSTC_VERSION", Some(87121)),
/// Allows using the `#[skip]` attribute in derives
(unstable, derive_skip, "CURRENT_RUSTC_VERSION", Some(121050)),
/// Controls errors in trait implementations.
(unstable, do_not_recommend, "1.67.0", Some(51992)),
/// Tells rustdoc to automatically generate `#[doc(cfg(...))]`.
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,5 +347,8 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di
"reduce the glob import's visibility or increase visibility of imported items",
);
}
BuiltinLintDiag::DeriveSkipUnsupported { traits } => {
diag.help(format!("the supported traits are {traits}"));
}
}
}

0 comments on commit 0d3c0c4

Please sign in to comment.