Skip to content

Commit

Permalink
Rollup merge of #101040 - danielhenrymantilla:no-bounds-for-default-a…
Browse files Browse the repository at this point in the history
…nnotated-derive, r=joshtriplett

Fix `#[derive(Default)]` on a generic `#[default]` enum adding unnecessary `Default` bounds

That is, given something like:

```rs
// #[default] on a generic enum does not add `Default` bounds to the type params.
#[derive(Default)]
enum MyOption<T> {
    #[default]
    None,
    Some(T),
}
```

then `MyOption<T> : Default`_as currently implemented_ only holds when `T : Default`, as reported by ```@5225225``` [over Zulip](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/.23.5Bderive.28Default.29.5D.20for.20enums.20with.20fields).

This is contrary to [what the accepted RFC proposes](https://rust-lang.github.io/rfcs/3107-derive-default-enum.html#generated-bounds) (_i.e._, that `T` be allowed not to be itself `Default`), and indeed seems to be a rather unnecessary limitation.
  • Loading branch information
matthiaskrgr committed Oct 3, 2022
2 parents c7f1b8e + 3d4980b commit df11395
Show file tree
Hide file tree
Showing 15 changed files with 77 additions and 2 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub fn expand_deriving_copy(
let trait_def = TraitDef {
span,
path: path_std!(marker::Copy),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: true,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub fn expand_deriving_clone(
let trait_def = TraitDef {
span,
path: path_std!(clone::Clone),
skip_path_as_bound: false,
additional_bounds: bounds,
generics: Bounds::empty(),
supports_unions: true,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub fn expand_deriving_eq(
let trait_def = TraitDef {
span,
path: path_std!(cmp::Eq),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: true,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub fn expand_deriving_ord(
let trait_def = TraitDef {
span,
path: path_std!(cmp::Ord),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub fn expand_deriving_partial_eq(
let trait_def = TraitDef {
span,
path: path_std!(cmp::PartialEq),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub fn expand_deriving_partial_ord(
let trait_def = TraitDef {
span,
path: path_std!(cmp::PartialOrd),
skip_path_as_bound: false,
additional_bounds: vec![],
generics: Bounds::empty(),
supports_unions: false,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub fn expand_deriving_debug(
let trait_def = TraitDef {
span,
path: path_std!(fmt::Debug),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/decodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub fn expand_deriving_rustc_decodable(
let trait_def = TraitDef {
span,
path: Path::new_(vec![krate, sym::Decodable], vec![], PathKind::Global),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_builtin_macros/src/deriving/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub fn expand_deriving_default(
let trait_def = TraitDef {
span,
path: Path::new(vec![kw::Default, sym::Default]),
skip_path_as_bound: has_a_default_variant(item),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down Expand Up @@ -262,3 +263,22 @@ impl<'a, 'b> rustc_ast::visit::Visitor<'a> for DetectNonVariantDefaultAttr<'a, '
}
}
}

fn has_a_default_variant(item: &Annotatable) -> bool {
struct HasDefaultAttrOnVariant {
found: bool,
}

impl<'ast> rustc_ast::visit::Visitor<'ast> for HasDefaultAttrOnVariant {
fn visit_variant(&mut self, v: &'ast rustc_ast::Variant) {
if v.attrs.iter().any(|attr| attr.has_name(kw::Default)) {
self.found = true;
}
// no need to subrecurse.
}
}

let mut visitor = HasDefaultAttrOnVariant { found: false };
item.visit_with(&mut visitor);
visitor.found
}
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/encodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub fn expand_deriving_rustc_encodable(
let trait_def = TraitDef {
span,
path: Path::new_(vec![krate, sym::Encodable], vec![], PathKind::Global),
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use std::cell::RefCell;
use std::iter;
use std::ops::Not;
use std::vec;
use thin_vec::thin_vec;
use ty::{Bounds, Path, Ref, Self_, Ty};
Expand All @@ -187,6 +188,9 @@ pub struct TraitDef<'a> {
/// Path of the trait, including any type parameters
pub path: Path,

/// Whether to skip adding the current trait as a bound to the type parameters of the type.
pub skip_path_as_bound: bool,

/// Additional bounds required of any type parameters of the type,
/// other than the current trait
pub additional_bounds: Vec<Ty>,
Expand Down Expand Up @@ -596,7 +600,7 @@ impl<'a> TraitDef<'a> {
cx.trait_bound(p.to_path(cx, self.span, type_ident, generics))
}).chain(
// require the current trait
iter::once(cx.trait_bound(trait_path.clone()))
self.skip_path_as_bound.not().then(|| cx.trait_bound(trait_path.clone()))
).chain(
// also add in any bounds from the declaration
param.bounds.iter().cloned()
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub fn expand_deriving_hash(
let hash_trait_def = TraitDef {
span,
path,
skip_path_as_bound: false,
additional_bounds: Vec::new(),
generics: Bounds::empty(),
supports_unions: false,
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/deriving/deriving-default-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ enum Foo {
Beta(NotDefault),
}

// #[default] on a generic enum does not add `Default` bounds to the type params.
#[derive(Default)]
enum MyOption<T> {
#[default]
None,
#[allow(dead_code)]
Some(T),
}

fn main() {
assert_eq!(Foo::default(), Foo::Alpha);
assert!(matches!(MyOption::<NotDefault>::default(), MyOption::None));
}
21 changes: 21 additions & 0 deletions src/test/ui/macros/macros-nonfatal-errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,24 @@ fn main() {

trace_macros!(invalid); //~ ERROR
}

/// Check that `#[derive(Default)]` does use a `T : Default` bound when the
/// `#[default]` variant is `#[non_exhaustive]` (should this end up allowed).
const _: () = {
#[derive(Default)]
enum NonExhaustiveDefaultGeneric<T> {
#[default]
#[non_exhaustive]
Foo, //~ ERROR default variant must be exhaustive
Bar(T),
}

fn assert_impls_default<T: Default>() {}

enum NotDefault {}

// Note: the `derive(Default)` currently bails early enough for trait-checking
// not to happen. Should it bail late enough, or even pass, make sure to
// assert that the following line fails.
let _ = assert_impls_default::<NonExhaustiveDefaultGeneric<NotDefault>>;
};
12 changes: 11 additions & 1 deletion src/test/ui/macros/macros-nonfatal-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,21 @@ error: trace_macros! accepts only `true` or `false`
LL | trace_macros!(invalid);
| ^^^^^^^^^^^^^^^^^^^^^^

error: default variant must be exhaustive
--> $DIR/macros-nonfatal-errors.rs:127:9
|
LL | #[non_exhaustive]
| ----------------- declared `#[non_exhaustive]` here
LL | Foo,
| ^^^
|
= help: consider a manual implementation of `Default`

error: cannot find macro `llvm_asm` in this scope
--> $DIR/macros-nonfatal-errors.rs:99:5
|
LL | llvm_asm!(invalid);
| ^^^^^^^^

error: aborting due to 27 previous errors
error: aborting due to 28 previous errors

0 comments on commit df11395

Please sign in to comment.