Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: non-defaultable structs #2019

Closed
YairHalberstadt opened this issue Nov 21, 2018 · 41 comments
Closed

Proposal: non-defaultable structs #2019

YairHalberstadt opened this issue Nov 21, 2018 · 41 comments

Comments

@YairHalberstadt
Copy link
Contributor

My proposal at #1981 wasn't very well received, so I've taken on some of the feedback, and come up with a new, simpler, proposal.

As an aside, I would like to assure you that although I call them non-defaultable structs you can still call default(T). What I really mean is non-uninitialisable structs. We'll get to that later.

Ok so onto the proposal.

Currently you can't declare a default constructor or field initialisers with structs.

This leads to a number of issues:

  1. It's impossible to make sure that all fields of a struct are correctly initialised. For example to make sure that reference type fields are not null.
  2. It's impossible to make construction of a struct private.
  3. If a struct has non-nullable reference type fields they will be null when the default constructor is used. The LDC does not have any good solution to this: see https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-08-09.md

Proposed Solution

The first step is obvious. Allow structs to declare default constructors and field initialisers.

A proposal #99 already exists to do this. However it lead to a lot of controversy because many developers thought default (T) and new T() should be the same.

My solution to this is relatively simple. We should do the same thing as we do for non-nullable reference types. Namely, for non-nullable reference types, the default value is an illegal value, so we force all non-nullable reference types to be initialised.

Similarly we should simply force all instances of a struct with a default constructor declared to be initialised.

If you don't want to initialise such a struct immediately, it must be declared nullable.

Assigning default(T) to such a struct should result in a warning, similar to the decision about non-nullable reference types in https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-08-09.md.

Syntax

I'm unsure about this. We could just say that a struct is non-uninitialisable if it declares a default constructor or a field initialiser.

However that is unobvious to the consumer, and makes it too easy for the library writer to make a breaking change without realising.

An attribute could be used, and a default constructor or a field initialiser can not be written unless the attribute exists. However thats not what attributes are for.

I feel like a new keyword is necessary. Perhaps something like

public nodefault struct Wrapper<T>
{
    public T Value {get;}
    private Wrapper<T>() => Value = default;
    public Wrapper<T>(T value) => Value = value;
}

Then usage would be like this:

public class Class
{
    Wrapper<int> Field1; //warns
    Wrapper<int> Field2 = new Wrapper<int>(0); //doesn't warn
    Wrapper<int>? Field3; //doesn't warn.
}
@quinmars
Copy link

Will the following warn?

Wrapper<int> val1 = Enumerable.Empty<Wrapper<int>>().FirstOrDefault();
var val2 = Enumerable.Empty<Wrapper<int>>().FirstOrDefault();

And will val2 be of the type Wrapper<int> or Wrapper<int>??

@ufcpp
Copy link

ufcpp commented Nov 21, 2018

I want default(T) and new T() are different.

@YairHalberstadt
Copy link
Contributor Author

@quinmars
https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-08-09.md discusses the type signature of FirstOrDefault.

Essentially, there may be another operator which indicates the default value of a type. So T? in an unconstrained generic would mean T? if T is a non-nullable reference type, T if it as a nullable reference type, T if it is a normal struct, and T? if it is a non-defaultable struct.

@YairHalberstadt
Copy link
Contributor Author

@ufcpp
Under this proposal they are. However at least the compiler warns you about using default

@DavidArno
Copy link

If/when we get discriminated unions (DUs), a means of marking a type (whether class or struct) as non-defaultable becomes pretty much a must. If I declare:

public enum struct Maybe<T>
{
    Some(T value),
    None()
}

then I'll want the compiler to ensure I cannot write:

Maybe<int> maybe = default;

as I'm leaving the struct in its default state, which makes no sense. That maybe must be None or Some<int>.

As we are getting flow control for nullable reference types, I'm hoping that control flow logic can be repurposed to not allow DU's to ever be default.

If such "non-defaultableness" can be achieved for DU's, then it seems only a small step to extend this to any type. Whether this happens via a keyword or attribute seems less important to me. It's the ability to mark a type of non-defaultable and have flow control enforce it that seems the important part. And by using flow control, we avoid the need for a CLR change too, which seems a big added bonus.

@YairHalberstadt
Copy link
Contributor Author

@DavidArno
We already have that for non-nullable reference types, so it's only structs that would need it

@DavidArno
Copy link

DavidArno commented Nov 21, 2018

Hmm, I was going to argue that NRE's only act on the instance, but I'd like a way of saying that the class itself should never be allowed to be null, ie for:

public nodefault class T {}

then T? would result in a warning.

But I now think there's a flaw in my thinking. Even a nodefault struct S can be marked S? as I'm asking for a nullable version of S, not its default. Having T? therefore be a warning creates inconsistencies. So I think I agree with you, it's only structs that need a way of marking as "no default".

The nagging doubt I now have though is what happens with a class-based DU? null there makes no sense to me still.

@YairHalberstadt
Copy link
Contributor Author

@DavidArno
Why not?

Either is a DU which either contains Left or Right.
Either? means I may not have set it yet to Left or Right.

@DavidArno
Copy link

To explain why I say null makes no sense for class-based DUs:

If I declare my DU as a class:

public enum class Maybe<T>
{
    Some(T value),
    None()
}

Then, when I pattern match on it:

Console.WriteLine(maybe switch
                  {
                      Some(var value) => $"Value is {value}",
                      None() => "No value"
                  });

then I don't want a warning that I've not covered the null-state and so the switch isn't exhaustive.

But there again, maybe the fact that maybe is declared as Maybe<int>, rather than Maybe<int>? means that again flow analysis can come to my aid by recognising that there shouldn't be a null so no null case is needed for the switch to be exhaustive. In which case that nagging doubt goes away.

@YairHalberstadt
Copy link
Contributor Author

@DavidArno

But there again, maybe the fact that maybe is declared as Maybe, rather than Maybe? means that again flow analysis can come to my aid by recognising that there shouldn't be a null so no null case is needed for the switch to be exhaustive. In which case that nagging doubt goes away.

Exactly my thoughts

@DavidArno
Copy link

@YairHalberstadt, I think I need to think things through before posting as that's twice now you've spotted the flaw in my argument and commented before I've had time to work out the flaw for myself 😆

@Ultrahead
Copy link

I want default(T) and new T() are different.

I agree. Maybe the solution should be allowing to define in valuetype class the default static value you want as a readonly. Then, if not assigned in the struct definition, the built-in default(T) would be used, instead (as usual, so as not to introduce breaking-changes). This way, default(T) and new T(), could be different.

@jnm2
Copy link
Contributor

jnm2 commented Nov 22, 2018

@DavidArno

as I'm leaving the struct in its default state, which makes no sense. That maybe must be None or Some<int>.

I can see this argument for a OneOf<T1, T2> type, but it makes a lot of sense to me for default(Maybe<>) to be a synonym for None.

@DavidArno
Copy link

@jnm2, surely Maybe<> is the special case here? Are there any other DUs where you'd want default to match a case?

@YairHalberstadt
Copy link
Contributor Author

@Ultrahead
Other than for performance, what possible advantage is there to having default (T) be different to new T()?
It's necessary currently because arrays and fields of structs and classes are zeroed out by default by the garbage collector.

@HaloFour
Copy link
Contributor

I want default(T) and new T() are different.

They already are. The latter will invoke a parameterless constructor if one exists. C# currently doesn't let you define a parameterless constructor on a struct, but that may change.

@jnm2
Copy link
Contributor

jnm2 commented Nov 23, 2018

@DavidArno certainly. I see DUs as an extension of enums. I don't have a problem with the default value of int or bool or any enum.

@orthoxerox
Copy link

@jnm2

What should be the default value of

public enum struct Result<T>
{
    Ok(T result);
    Error(Exception ex);
}

@DavidArno
Copy link

@jnm2,

I see DUs as an extension of enums. I don't have a problem with the default value of int or bool or any enum.

Ah well, maybe that gets to the heart of it. false is only the default boolean value because the CLR defaults everything to zero. It makes no more sense - beyond implementation details - for false to be the default state than for true to be the default state. I'd argue that, ideally, neither should be: you should always have to pick a state.The same applies to all enums in my view.

Likewise, why is 0 the default for an int? It's an ordered value, so why not default to the first number, -2,147,483,648? Again, it's an implementation detail as everything is zero-filled by default.

So I'd be unhappy with this same "implementation detail" artefact of default values then being applied to DU's, which are truly nominal data in most cases, just because everything else to date also has a default state.

@HaloFour
Copy link
Contributor

I'd suggest that there really isn't a good solution to the "default value" problem, certainly not without dragging CLR changes into the mix which I imagine would spin out for a bit. So in my opinion it's worth having the conversation as how to design DUs in such a way to try to avoid the problems that will result in a zeroed-out default. From a language perspective it's probably reasonable to treat DUs like enums, where you can have a degree of confidence that the DU will be one of the enum values but where there is no guarantee.

@DavidArno
Copy link

As @YairHalberstadt has pointed out, when v8 lands, reference types will no longer have a default state, unless you explicitly mark them as nullable. This instantly changes the argument. No longer must everything have a default.

Extending this "no default state" to opt-in structs makes a lot of sense. And it neatly addresses the only real opposition to having C# directly support default and new() being different things for structs (the fact that the constructor is not called for default). For those cases where both new() and default currently cause problems, we can exclude the latter and provide an implementation for handling the former.

Having landed in this position (if we did), then having the norm for DU's be "non defaultable", unless explicitly overridden, makes complete sense to me.

@HaloFour
Copy link
Contributor

@DavidArno

Something like that would probably work fine, require a value on initialization. There's still no guarantee, and you still have the gaping holes with array initialization and generics, but hopefully that would be sufficient.

@jnm2
Copy link
Contributor

jnm2 commented Nov 23, 2018

@DavidArno

I'd argue that, ideally, neither should be: you should always have to pick a state.The same applies to all enums in my view. […] Again, it's an implementation detail as everything is zero-filled by default.

I've long valued the specific convenience of defaulting value types: I don't have to initialize fields to 0 or MyState.NotStarted or false, the most common initialization values.
Even more to the point, I don't see a significant downside to picking the most common initialization value as the value to which fields and arrays are implicitly (and cheaply) initialized. You're saying it makes no sense, but I see the sense of it and I think it's nice to have.

@YairHalberstadt
Copy link
Contributor Author

@jnm2
It happens to be that these are decent defaults for some structs. For others though, they really aren't ideal. For example a default ImmutableArray is an ImmutableArray wrapping a null Array, which is far from ideal.

@jnm2
Copy link
Contributor

jnm2 commented Nov 23, 2018

@YairHalberstadt Exactly, and this is particularly painful in the case of OneOf<T1, T2> as I mentioned. Just not all value types, and I would have said Maybe<> was a great example of where having a default makes a lot of sense. Similar to how it makes sense that new List<>() starts empty.

@Ultrahead
Copy link

@Ultrahead
Other than for performance, what possible advantage is there to having default (T) be different to new T()?
It's necessary currently because arrays and fields of structs and classes are zeroed out by default by the garbage collector.

I guess @YairHalberstadt that you answer it your-self ...

@jnm2
It happens to be that these are decent defaults for some structs. For others though, they really aren't ideal. For example a default ImmutableArray is an ImmutableArray wrapping a null Array, which is far from ideal.

@CyrusNajmabadi
Copy link
Member

For example a default ImmutableArray is an ImmutableArray wrapping a null Array, which is far from ideal.

I disagree. This is exactly what i want as it means a default immutable array behaves the same as a default normal array.

@svick
Copy link
Contributor

svick commented Nov 23, 2018

@CyrusNajmabadi

This is exactly what i want as it means a default immutable array behaves the same as a default normal array.

Except it doesn't, because it can't. Sure, a == null is true. But object.Equals(a, null) and object.ReferenceEquals(a, null) are both false. So you trade one kind of consistency for another kind of consistency and I'm not sure I like that trade-off. (Though I believe ImmutableArray<T> has this design because of performance concerns, not consistency.)

@YairHalberstadt
Copy link
Contributor Author

YairHalberstadt commented Nov 24, 2018

@Ultrahead
But there I would want default (ImmutableArray) to be the same as new ImmutableArray().

I would never want default(T) to be a non zeroed out struct and new T() to be a different non zeroed out struct!

@Austin-bryan
Copy link

Wouldn't changing what default(T) does be a breaking change? Unless the proposal is suggesting having a syntax to have different structs that allows you to have them seperate.

Frankly it is kind've silly to have default(T) and new T() do the exact same thing.

@YairHalberstadt
Copy link
Contributor Author

@Willard720
I think you have misunderstood the proposal. It is about using flow analysis to make sure that non-defaultable structs are always initialised, similar to non-nullable reference types, not about changing the meaning of default

@gafter
Copy link
Member

gafter commented Nov 25, 2018

But there again, maybe the fact that maybe is declared as Maybe<int>, rather than Maybe<int>? means that again flow analysis can come to my aid by recognising that there shouldn't be a null so no null case is needed for the switch to be exhaustive. In which case that nagging doubt goes away.

This is our plan. See dotnet/roslyn#30597

@MkazemAkhgary
Copy link

instead of nodefault keyword, special attribute could be used, say [NoDefault].

@YairHalberstadt
Copy link
Contributor Author

@MkazemAkhgary
I think that's a good idea if you can write a default constructor on a normal struct.
However if you can only write a default constructor on a nodefault struct, it should be part of the language, and so should require a keyword not an attribute.

@Ultrahead
Copy link

Ultrahead commented Nov 26, 2018 via email

@jnm2
Copy link
Contributor

jnm2 commented Nov 26, 2018

You'd have to prevent nodefault structs from being used as type parameters. This means that a wrapper struct NoDefaultWrapper can't implement (e.g.) IEquatable<NoDefaultWrapper>.

The reason WrapperStruct can't be used as a generic type parameter is that otherwise nodefault is ineffective:

public void GetDefaultValue<T>() => default(T);

Console.WriteLine(GetDefaultValue<NoDefaultWrapper>()); // !

The above code is blatant, but there are plenty of ways that nodefault could be remotely rendered ineffective.

The only solution that I can see is a CLR change introducing a new generic constraint default or defaultable:

// New compile error (warning?)
public void GetDefaultValue<T>() => default(T);

// Works
public void GetDefaultValue<T>() where T : default => default(T);

Which is a breaking change and therefore should not happen.

@YairHalberstadt
Copy link
Contributor Author

@jnm2
The same problem is discussed in https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-08-09.md for non nullable reference types

@YairHalberstadt
Copy link
Contributor Author

They have no good solution for this.

Essentially, I think the best we can do is add an analyser which suggests adding an attribute to a generic method which calls default(T), and warn when using a non-defaultable type as a type parameter for such a method.

@YairHalberstadt
Copy link
Contributor Author

Changing default T in a generic setting to mean a nullable T, as suggested in that LDM, works for non nullable reference types as-is, but would require CLR changes for nodefault structs.

However, since the Jitter creates a new version of a generic type for every struct, the CLR changes aren't that drastic. However it would essentially restrict this to a CoreCLR only feature.

@YairHalberstadt
Copy link
Contributor Author

#146

@YairHalberstadt
Copy link
Contributor Author

Seeing as #146 is championed, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests