Hello,
For my first contribution I was thinking implementing Default for std::time::Instant with Instant::now(). I read the contributing guidelines carefully but I still have a few questions and would appreciate your comments please :
Thanks in advance for your help !
You can't feature guard trait impls, they are insta-stable. Just open a PR with the change, I think someone on the lang team can help you once you've done that.
I'm not sure how I feel about this - it would I think be the first nondeterministic Default implementation.
Agreed with @sfackler -- I think it is a reasonable expectation from the standard library (probably does not hold in the wider ecosystem) that default() is deterministic because default() is expected to be mundane. Instant::now() does not feel onerous in this case.
RandomState is another with a nondeterministic default, which also affects HashMap and HashSet if you're using their default S = RandomState type.
Thank you all for you comments. I understand the deterministic concern but I am afraid there are no deterministic value for std::time::Instant. Do you believe it is worth submitting a PR ? I will also implement default for std::time::SystemTime with SystemTime::now().
This idea has been tried before and was closed after a FCP. I'm not sure whether you'll be able to gather more support this time, unless you have some stronger motivation. Just being a (the?) reasonable way to build a std::time::Instant out of nowhere is not sufficient it seems.
Thinking more about it, I don't think I know of an API that accepts a generic T: Default and where I would find it reasonable to use Instant::now(), but I am not a big user of timing APIs.
I am so sorry i missed this PR. I searched the issues but forgot to look for PRs. I understand that none deterministic default where Default::default() != Default::default() could return true is an issue.
Reading the full history I believe my concern is more the pain to implement Default on a struct where almost all fields implement Default but one or two. This should be addressed by this draft RFC but I am not sure it is still being worked on.
Since I am not bringing anything new on the table I will not submit a PR.
Thank you very much to all of you for your time and comments. I guess my first contribution is just a matter of time !
I would expect Instant::default() to return a constant instant, probably the start of the Unix epoch, and certainly NOT Instant::now().
In fact, I think implementing it in terms of Instant::now() is completely wrong, because if a structure has multiple default instants, they must be equal and also there must be at most one system call to get the time, and such a default implementation does not meet those requirements.
Option<Instant> is a great option for that. You might be able to make a PR to make an Instant's implementation hold a NonZero of some sort to eliminate the space overhead of that.
That might make sense for SystemTime, but Instant isn't related to any consistent epoch.
This seems like a really strange recommendation. It seems clear this user would want the default value to be now, not a null? I would never make a value nullable to be able to derive Default instead of manually implementing it, that seems like a definite programmer error.
Does it? Without context it's hard to say, but @scottmcm's suggestion to make Instant non-zero makes a lot of sense to me (having worked with simulation time points using a special value never). It's also not unusual to want to construct structs where some fields do not get used until later, but per language rules still need some initialiser.
I was actually kind of surprised by this thread... In my mind, Default means "do the obvious thing", and to me Instant::now() is the only obvious thing one would do; at least, it's the only thing I have ever done with it...
This is what I would expect as well: a consistent default value every time. I would have no problem with an implementation of Default, as long as it defaults to the same value every time.
I don’t have a preference here, but I do want to raise the point that Default should have its docs clarified. It should either explicitly encourage implementations to use a deterministic default or explicitly discourage users from assuming all implementations are deterministic (or both). Many implementations are deterministic but I think the docs could provide additional guidance (and that guidance might help steer this conversation).
It's probably worth noting that all current std Default implementations construct an "empty" or "zero" value; a value that holds no real information. Though HashMap and friends are nondeterministic, that's entirely internal to the API; the external appearance is that HashMap::default gives an empty hashmap every time, with no observable difference. (After all, the iteration order is arbitrary!)
While it isn't "wrong" to say Default::default is the obvious value (the empty/zero/null value is the canonical one), I think it's not quite specific enough. While it doesn't require it, Default to me has the implication of "empty", where the default value doesn't hold any information, it's just ready to hold information that the caller gives it.
I don't really know how that can be formalized into the docs, but I think that idea of a "blank" return value rather than something around determinism is the right specification to make.
Default means many things, ranging from the minimum 'instantiate an instance of this type', to the various properties this instance should have.
Various other Default discussions on this forum:
Add is-default autimatically to types that implement Default and PartialEq
The docs state:
A trait for giving a type a useful default value.
might imply a value that later can be actually used
Sometimes, you want to fall back to some kind of default value, and don’t particularly care what it is .
which is just an instance. It might in the extreme case even be a random one and differ between invocations. I have seen this be preferred in the usecase of deserializing / constructing larger structs, using default() like you would mem::uninitialized() (before MaybeUninit), only difference being that the provided value is actually a valid instance.
Already these definitions are sometimes at odds with eachother, as in the discussions about if null or dangling are "useful" defaults for pointers an NonNull, since they can never actually be used.
Further properties of Default I have seen, some already mentioned in this thread:
It should be a single deterministic/canonical value (would enable is_default).
It should be an empty value: 0, false, None, "", empty collections, null?
It should actually be a default value, conceptionally equivalent to calling ::new(): the default with no extra configuration.
Instant::now()?It should be just an instance, with the extra guarantee of minimal resource usage: vec![1, 2, 3] as default requires an allocation and is not okay, but 7 is a valid default for usize.
These differences are confusing, and usage of Default and the expectations about properties differs between use cases, types and people. Furthermore, most of these properties are indeed useful to have, but not necessarily applicable in all contexts. To me at least it feels like these should not all be collected under the same trait, especially under a name like "Default", which itself implies more that the docs state.
Field defaults would solve every case where I might have wanted something like this, and seems like it would put an end to requests for things to have surprising/nondeterministic/expensive/frankly dangerous defaults. I don't think it's a problem that default means fuzzily different things to different types. Field defaults seems like a fairly principled approach to the inconvenience.
My thoughts exactly. Default is simply too coarse a tool in cases like this, and it feels like an instance of everything looking like a nail if all you have is a hammer.
If I understand everything correctly Instant::now() is not a const function (and I think can't be, since it may return a different value on every call), so it couldn't be used with Field Defaults as specified in the linked draft RFC.
I personally am a fan of the idea that Default values are invariant across runs of the program etc. I am uncomfortable that there is a default for RandomState, although I understand the use in hashing and don't have a great alternative to suggest.