-
Notifications
You must be signed in to change notification settings - Fork 1k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Championed: Non-defaultable value types #146
Comments
As for me there shouldn't be non-defaultable structs at all. It's too dangerous thing as it is impossible to guarantee the struct is initialised properly. There're too much corner cases to check. I'm not speaking about trivial cases, such as struct NonDefaultable
{
private int _x;
public NonDefaultable(int x)
{
if (x == 0)
throw new ArgumentException(nameof(x));
_x = x;
}
public int X
{
get
{
if (_x == 0)
throw new InvalidOperationException("Boom!");
return _x;
}
}
}
delegate void OutCallback(out NonDefaultable x);
static void NeverFinishes(out NonDefaultable a)
{
while (true)
{
Thread.Sleep(TimeSpan.FromDays(1));
}
}
static void Main(string[] args)
{
OutCallback a = NeverFinishes;
a.BeginInvoke(out var temp, null, null);
var result = temp;
// ...
// somewhere in another method, hours later
Console.WriteLine(result);
Console.WriteLine(result.X); // happy debugging!
Console.ReadKey();
Console.WriteLine();
Console.WriteLine("Done.");
} The worst thing is that there's nothing like As for me it's pretty obvious thing and I'm very worried about the fact the idea is proposed by C# team. There's no good reason behind this but 'symmetry with reference types'. Okay, let's look at real-world use cases. There's attempt to add non-defaultable type into framework, an Worst decision ever 👎 |
Can such types be used as type arguments to unconstrained generic type parameters? |
Does your concern also apply to non-nullable reference types?
This is a problem of |
@gafter |
@ufpp You didn't answer my question. I'm not asking about new constraints. I'm asking if such types can be used as a type argument for an unconstrained type parameter. |
@gafter |
It doesnt and there are at least two reasons: Second, it's a well-known issue and existing codebase deals with it nicely. The proposal (to be precise, the #99 one) will lead to similar issue that will not and may not be handled by runtime. It may not and will not be handled by existing code - no one will perform code review just to proof it works fine with default(NonDefaultable). This means there's no way to make BCL structs to be non-defaultable without introducing a huge breaking change. And this (finally) means there will be two set of value types: first, most common, supports default(T) and does not throw when such value is used. And second one, just added, has no benefit but throwing when you do not expect it. Dislike it.
Yep. This means that any existing code may suffer from it. It's not only UPD
Missed this one, sorry :) Does it mean that IEnumerable, lists, linq extension methods, tasks and so on cannot be used together with non-defaultable value types? |
@ufcpp That seems to be a violation of the principle that type parameter constraints only narrow the set of possible values for a type parameter. If/when this feature is added to the compiler, how do you imagine an earlier version of the compiler will prevent someone from using such a type as a type argument for an unconstrained type parameter? |
I imagine non-defaultable value types could be treated as the same as nullable reference types: For nullable reference types: using System.Text;
class NullableReferenceTypes
{
static void Main()
{
var s1 = "ᛉ🐕𓀀";
var s2 = default(string);
var f1 = F(s1); // OK
var f2 = F(s2); // OK
var g1 = F(s1); // OK
var g2 = F(s2); // warning
string traditional = CompiledInEarlierVersion<string>.Item; // warning
string nonNullable = CompiledInFutureVersion<string>.Item;// OK
string? nullable = CompiledInFutureVersion<string>.NullableItem;// OK
}
// nullable
// compiled as `int? F([Nullable] string s)`
static int? F(string? s) => s?.Length;
// non-nullable
static int G(string s) => s.Length;
}
// in the old/opt-out world
class CompiledInEarlierVersion<T>
{
public static T Item => default(T); // treated as "traditional" reference from the new/opt-in world
}
// in the new/opt-in world
class CompiledInFutureVersion<T>
{
public static T Item => default(T); // warning
public static T? NullableItem => default(T); // OK?
} For non-defaultable value types: using System.Text.Utf8;
class DefaultableValueTypes
{
static void Main()
{
// default(Utf8String) is implemented as an empty string in System.Text.Primitive
// but assume Utf8String were a non-defaultable value type in this code.
var s1 = new Utf8String("ᛉ🐕𓀀");
var s2 = default(Utf8String);
var f1 = F(s1); // OK
var f2 = F(s2); // OK
var g1 = F(s1); // OK
var g2 = F(s2); // warning
Utf8String traditional = CompiledInEarlierVersion<Utf8String>.Item; // warning
Utf8String nonNullable = CompiledInFutureVersion<Utf8String>.Item;// OK
Utf8String? nullable = CompiledInFutureVersion<Utf8String>.NullableItem;// OK
}
// defaultable
// compiled as `int? F(Utf8String s)`
static int? F(Utf8String? s) => s?.Length; // s != default ? s.Length : (int?)null;
// non-defaultable
// compiled as `int? F([NonDefault] Utf8String s)`
static int G(Utf8String s) => s.Length; // s != default ? s.Length : throw new NullReferenceException();
}
// in the old/opt-out world
class CompiledInEarlierVersion<T>
{
public static T Item => default(T); // treated as "traditional" value from the new/opt-in world
}
// in the new/opt-in world
class CompiledInFutureVersion<T>
{
public static T Item => default(T); // warning
public static T? NullableItem => default(T); // OK?
} |
I got your point. Well, non-defaultable value type is more dangerous than nullable reference types. However I think its danger is not so fatal. There is a trade-off between safety and performance. This feature is a bit dangerous but has merit in performance. namespace WhatCurrentlyRecommended
{
// default(PositiveInt) is a valid value: 1.
// safe but slow
struct PositiveInt
{
private int _value;
public int Value => _value + 1; // extra ADD instruction for each access
public PositiveInt(int value)
{
if (!Validate(value)) throw new InvalidOperationException();
_value = value - 1;
}
public static bool Validate(int value) => value > 0;
}
}
namespace WhatIWant
{
// I want default(PositiveInt) to get warning.
// dangerous but fast
[NonDefaultable]
struct PositiveInt
{
public int Value { get; }
public PositiveInt(int value)
{
if (!Validate(value)) throw new InvalidOperationException();
Value = value;
}
public static bool Validate(int value) => value > 0;
}
}
I don't want all structs to be non-defaultable. It's ok if only structs opted-in are non-defaultable. |
Aaand for sake of perfomance we now have a type in a public API that implements IEnumerable, throws and provides no way to check if it is safe to enumerate (casting to This means that any public API that consumes |
This is also useful for a
|
Turned this discussion into a championed issue. Thanks for the proposal. |
In an informal discussion on gitter with @CyrusNajmabadi, he felt that the main type which would gain from this feature was ImmutableArray. He also felt that since it was too late to fix ImmutableArray, the advantage of this proposal was limited. I wonder if there would be any way to allow ImmutableArray to make use of this, by tying in this feature with NRT. So using default or new on an ImmutableArray would be an error when nullables are enabled, but not when they are disabled. Alternatively, I guess since ImmutableArray is provided via a nuget package, it would probably be possible to make a breaking change to it, and increment the major version of the package. This would cause issues though if two libraries reference different versions of it, if the constructor is changed. |
Why would you make this an error? A default ImmutableArray is a totally reasonable and expected value. It's something code wants to be able to use. You want non-defaultable types for cases where the 'default' value is simply something that should never come into play, and basically will always be a problem. ImmutableArray is just not in that category. |
If we support no-arg constructors in structs, we could possibly implement this as a warning when defaulting such a type. |
@gafter I was thinking about this today, and I believe the following high-level items could bring this feature up to the current level of reference types:
|
I just had an idea I wanted to throw into this discussion. MotivationNRT flow analysis will already track when you've done comparisons of a given variable with Consider the following, given a if (someS == default(S)) What stands out to me is that:
So I propose an additional attribute, similar to APInamespace System.Diagnostics.CodeAnalysis
{
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, Inherited = false, AllowMultiple = false)]
public sealed class IsInitializedWhenAttribute : Attribute
{
public IsInitializedWhenAttribute(bool returnValue);
}
} UsageThe most well-known example case for this (and #99) is [NonDefault] //added to disallow use of 'default' directly
public partial struct ImmutableArray<T>
{
[IsInitializedWhen(false)] //added to signify this property can participate in the flow analysis
public bool IsDefault { get; }
[IsInitializedWhen(false)] //either one or the other can be checked safely
public bool IsDefaultOrEmpty { get; }
} Also, adding to our example struct [NonDefault]
partial struct S
{
[IsInitializedWhen(true)]
public bool IsInitialized { get; }
public override int GetHashCode()
{
if (!IsInitialized) throw new InvalidOperationException("Instance not properly initialized");
//...
}
} In usage: void M(ImmutableArray<S>? nullableImmutableArray)
{
var arr = nullableImmutableArray.GetValueOrDefault(); //it's not possible to outright prevent a 'default(T)' escaping when the return value is a generic,
//but annotations will tell the flow analysis that said return is possibly 'default'
var s = arr.FirstOrDefault(); //Warning: Possibly operating on a default instance of a non-defaultable value type, this operation could throw at runtime
if (!foo.IsDefault) //the annotated properties are exempt from the warning because that's their purpose
{
s = arr.FirstOrDefault(); //no warning because the flow analysis is satisfied
var hash = s.GetHashCode(); //Warning again
if (s.IsInitialized)
{
hash = s.GetHashCode(); //fine again
}
}
} Additional thoughts
AlternativesWhile not mutually exclusive, another approach that came to mind while I was writing this was mirroring the //operator
public static bool operator ==([AllowDefault] S left, [AllowDefault] S right);
//IEquatable<T>
public bool Equals([AllowDefault] S other); and passing |
Non-defaultable value types will fail in a lot of cases, like when I can see why fields of value types cannot be non-nullable, but why not have this? struct Wrapper<T> where T : class
{
public T Value { get; } = throw new InvalidOperationException("The value must be initialized.");
public Wrapper(T? value)
{
Value = value ?? throw new ArgumentNullException(nameof(value));
}
} The compiler would generate a new field, let's say This gets you the best of both worlds. The struct is effectively non-defaultable, as any default value is guaranteed to be invalid in relevant contexts, but the property is guaranteed to be non-nullable (when the call to And if #99 gets through, |
This same caveat already exists when you do |
Nowadays I work more with structs than classes. I often need to warn the user that a struct must be initialised with the proper constructor and even if I declare T as a field and I decided to NOT initialise it, it's still an explicit intent. At most a warning may be useful, but I don't think so. However, it's true that I am thinking only about my use cases, so I may be wrong for other cases. |
I've been reading a similar thread at #5075 I'd love to see immutable, not-defaultable, 'value objects' in the language. I mentioned two things in that thread. [ValueObject(typeof(int))]
public partial struct CustomerId {
// optional
private static Validation Validate(int value) => value > 0
? Validation.Ok
: Validation.Invalid("Customer IDs must be a positive number.");
} Usage looks like: var id1 = CustomerId.From(123);
// error VOG009: Type 'CustomerId' cannot be constructed with default as it is prohibited.
CustomerId c = default;
// error VOG009: Type 'CustomerId' cannot be constructed with default as it is prohibited.
var c2 = default(CustomerId); ... and if we try to circumvent validation by adding other constructors, we get: [ValueObject(typeof(int))]
public partial struct CustomerId {
// Vogen already generates this as a private constructor:
// error CS0111: Type 'CustomerId' already defines a member called 'CustomerId' with the same parameter type
public CustomerId() { }
// error VOG008: Cannot have user defined constructors, please use the From method for creation.
public CustomerId(int value) { }
} I'd love some feedback on this, but I'd what I'd love more is such a concept to be in C#! Which brings me onto my next comment from that other thread (I'll put that as a new comment below) |
Maybe if the language had an invariant (or similar) modifier in the primary constructor? After generating the primary constructor, the compiler could write instructions to call each public record struct CustomerId(invariant int Value)
{
// the argument name and type must match an argument name and type from the primary constructor
private static void Validate(int Value) => Invariant.Ensure(Value > 0);
// compiler error: CS4242 - No matching invariant in primary constructor named Oops
private static void Validate(string Oops) => Invariant.Ensure(Value > 0);
} The Invariant type could use Caller Argument Expressions... Something like this: public readonly struct Invariant
{
public Invariant And(bool condition, [CallerArgumentExpression("condition")] string? expression = null) =>
Ensure(condition, expression);
private static Invariant Ensure(bool condition, [CallerArgumentExpression("condition")]string? expression = null)
{
if (!condition) throw new Exception($"Invariant failed, expected: {expression}");
return default;
}
} So a We could also have compiler analysis that says that 'any type with a primary constructor that has an |
It may be explicit to you at the time of writing that field, but not to anyone else reading it later on. |
what I meant is that not initializing the struct is not different than using default(T), however c# does throw compiler errors if I don't initialise the struct properly before using, so in reality what I wrote cannot be done anyway. The whole point was about that new T() and default(T) could be treated differently (and the compiler treats them differently already as far as I can see). In this way I can stop users from using new T(), but I would never stop them from using default(T). This behaviour/limitation would solve my code design issues, as the user would be aware that using new T() is not expected. Note: I think I got to this post because I have been told that my issue is the same of this thread, I am not so sure anymore. I have the similar need to force the user to use specific constructors, but my problem is not about nullable values, is code design in general, which is the impossibility to communicate to the user my intent of not allowing them to use default constructors on struct. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Ported from dotnet/roslyn#15108
Non-defaultable value types
Summary
Certain initialization is necessary for Method Contracts, especially Nullability checking. However, if
T
is a struct,default(T)
makes all members 0 or null and skips certain initialization. Thus, flow analysis is needed for not only contracts and nullability checking for reference types but also "defaultability" checking for value types.Motivation
Example 1: Thin Wrapper of Reference
From the viewpoint of performance, we sometimes need to create a thin wrapper struct which contains only few members of reference types, e. g.
ImmutableArray
. Now that performance is one of big themes in C#, needs of such a wrapper struct would increase more and more.Given the code such as:
If Records and nullability checking would be introduced to C#, this could be shortly written as the following:
T
is non-nullable from now, and useT?
if null is required. The problem here is that an instance of this struct can contain null by usingdefault(Wrapper<T>)
althoughT Value
is supposed to be non-nullable.Example 2: Value-Constrained Structs
Suppose that you implement a type which value has some constraints: for instance, an integer type which is constrained to be positive:
If Records and Method Contracts would be introduced to C#, this could be shortly written as the following:
This looks good at first glance, but,
default(PositiveInt)
makes 0 which is an invalid value.Proposal
Nullability checking proposed in dotnet/roslyn#5032 is based on flow analysis. Defaultability checking is essentially the same analysis as the nullability checking and could be implemented by the same algorithm.
However, in most case,
default(T)
is a valid value ofT
. Therefore, some kind of annotation would be required. For instance, how about using a[NonDefault]
attribute on structs.I call these types "non-defaultable value types". As with non-nullable reference types, defaultablity should be checked by using flow anlysis.
If
T
is a non-defaultable value type,T?
doesn't requireNullable<T>
becausedefault(T)
is an invalid value and can be used as null.Moreover, ordinary value types should not be allowed to have members of non-nullable reference types. Only types that be allowed it would be reference types and non-defaultable value types.
The text was updated successfully, but these errors were encountered: