Rollup merge of #101040 - danielhenrymantilla:no-bounds-for-default-annotated-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.
This commit is contained in:
Matthias Krüger 2022-10-03 20:58:55 +02:00 committed by GitHub
commit df11395a55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 77 additions and 2 deletions

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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,
@ -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
}

View File

@ -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,

View File

@ -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};
@ -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>,
@ -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()

View File

@ -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,

View File

@ -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));
}

View File

@ -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>>;
};

View File

@ -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