Implement Default for &Option

[camsteffen]

Pretty straightforward, but here is some justification:

I wasn't able to derive Default for my struct containing a &Option<MyType> because I can't implement Default for &Option<MyType> because of the orphan rule. I suppose this is an unusual case because Option<&T> is generally preferred over &Option<T>. But I think adding this is justified for a couple reasons. 1) It removes an unnecessary speed bump. 2) I think there are times when &Option<T> is preferable. In my project I am creating and passing around &Option<MyType> in a lot of places, but only actually using MyType in one place. So it's not worth the ergonomic cost of having to write build_mytype(...).as_ref() instead of &build_mytype(...) in many places.

r? libs

[jieyouxu]

r? libs-api

[camsteffen]

Looking at this more closely, this is not as obvious as I thought. There doesn't seem to be any precedent in std for implementing Default for any reference types excluding unsized types like &str. Is this a slippery slope of adding impl Default to references to all std types? Or is there an obvious solution I'm missing? Also I realized that feature(default_field_values) would be good enough for the use case I encountered. So I'm unsure, but still wonder if adding this is worth it because of the ubiquity of Option and the orphan rule. (I probably should have started a discussion instead of a PR, apologies)

[joshtriplett]

I'm honestly surprised that this works at all. Assuming it does work, it seems reasonable to add.

[tgross35]

I wonder, would it be possible to make the derive smarter to handle all reference types? Rather than needing to add this on a case-by-case basis.

[joshtriplett]

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

Concerns:

• const-trait (Implement Default for &Option #140808 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Amanieu]

I'm concerned about the forward-compatibility of this with a more general Default impl for all T: const Default (using const traits). Specifically, this would probably at least require an additional T: Freeze and T: !Drop bound, which would make the future impl incompatible with this one.

@rfcbot concern const-trait

[scottmcm]

ymmv: I'm pretty sure you can remove the explicit lifetime here by writing it as

Suggested change

	impl<'a, T> Default for &'a Option<T> {
	/// Returns `&None`
	#[inline]
	fn default() -> &'a Option<T> {
	&None
	}
	}
	impl<T> Default for &Option<T> {
	/// Returns `&None`
	#[inline]
	fn default() -> Self {
	&None
	}
	}

[traviscross]

Regarding the concern about forward-compatibility with what we might want with const traits, a Freeze and !Drop bound, etc.:

cc @oli-obk @fee1-dead @compiler-errors @p-avital

[oli-obk]

This could be experimented with when #134628 lands (it's already approved)

[camsteffen]

Specifically, this would probably at least require an additional T: Freeze and T: !Drop bound

I think that impl should not exist because of those constraints? This impl on &Option<T> does not have any constraints on T because the compiler knows that the expression None does not contain a T.

[tmke8]

I think the point is to add an additional (though in this specific case unnecessary) bound on the impl so that in the future, a more general impl can still be added that is still compatible with this one. (The more general impl would implement Default for all &T where T has a default that can be generated with const code, as is the case for Option<...> where the default is just a None.)

[camsteffen]

I feel like I'm missing something here. Isn't it not possible to add an impl for all &T because that would break any user's impl for &WhateverType?

[dtolnay]

No. No user-defined type currently implements const Default so adding impl<T: const Default + ???> Default for &T would not overlap with any user impl.

[camsteffen]

I see. Thanks for spelling that out. 😅

Back to my previous comment then, if we add impl<T: const Default + Freeze> const Default for &T, that would prevent us from writing:

fn test() -> &'static Option<MyType> {
    Default::default() // error: not MyType is not Freeze
}

#[derive_const(Default)]
enum MyType {
    Foo(Cell<u32>),
    #[default]
    Bar
}

[Amanieu]

Restarting FCP to reflect updated team membership.

@rfcbot cancel
@rfcbot merge

[rust-rfcbot]

@Amanieu proposal cancelled.

[rust-rfcbot]

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @the8472

Concerns:

• const-trait (Implement Default for &Option #140808 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Amanieu]

Re-adding concern:

I'm concerned about the forward-compatibility of this with a more general Default impl for all T: const Default (using const traits). Specifically, this would probably at least require an additional T: Freeze and T: !Drop bound, which would make the future impl incompatible with this one.

@rfcbot concern const-trait