Champion "Nullable reference types" (16.3, Core 3) #36
Comments
|
Will default constructor also generate warnings for all non-initialized reference type members? |
|
In the following case: I think that in both cases it should be |
Can we have a serious discussion about using modopts (or even modreqs) to allow for nullability overloads, i.e. two methods who only differ by nullability? This would be useful for functions where null input = null output, and non-null input = non-null output, or even vice-versa. |
|
I believe that conversation has happened a couple of times already. |
|
I think this feature is about objects and not types as it does not change anything about the actual type system. When nullability was introduced for value types, it was about the types. But that's not the case here. Quoting the proposal (emphasis mine)-
I think the "variables, parameters and results" (are the members of a class being excluded here?) can be easily replaced by "objects". The proposal is introducing non-nullable objects(even including value types) and putting them to be default with separate notation for nullable objects. So, the proposal can be renamed to- |
|
@gulshan Objects exist at runtime. Types exist at compile-time. This is about changing what happens at compile-time, not at runtime. |
|
@gafter I have two questions-
For nullable value types, both the answers are "no". But, I guess for nullable references, the answer is "yes". Because there is nothing changed about types. Only thing changed is, whether a reference should hold null or not. If I am wrong here, please correct me. BTW, now I propose the title "Default to non-nullable references" or simply "Non-nullable references". |
|
@gulshan Yes, and yes. The first because |
|
Forgot to ask that. Is this feature would allow generic nullable type? I mean code like this should be able to compile with this feature enabled? public T? GET<T>(string obj)
{
try { return HttpGet(url).Convert<T>(); }
catch { return null; }
}
int n = GET<int>(url0) ?? -1;
var m = GET<MyCustomClass>(url1); |
|
@Thaina Not as currently envisioned, no. There is no way to translate that into something expressible in the CLR (except by duplicating the method for each "nullable" unconstrained type parameter). |
|
@gafter how would duplicating work if C# can't support methods that have mutually exclusive constraint flags (gpReferenceTypeConstraint and gpNotNullableValueTypeConstraint) but have otherwise identical signatures from the viewpoint of overload resolution? |
|
The proposal doesn't take into considerations any of the discussion from the roslyn repo about telling the compiler "I know at this point in time that the value is not null. Do not warn me.", for example using a Can we take that into consideration, or will |
|
@gafter In @Thaina 's example, would it be appropriate to lift the return into a I was thinking that the original example: would like this if you decompiled it: public Nullable<T> GET<T>(string obj)
{
try { return HttpGet(url).Convert<T>(); }
catch { return null; }
}But in C# vNext you wouldn't know. The type just appears to be |
|
I'm still a bit curious how this could possibly be implemented with var s = "abc";
...
s = null;This is perfectly valid currently. However, what type should |
|
@MikeyBurkman it's may be inferred in different ways based on project checkbox (like it's done for |
|
To my mind the most important issue, by far, to fix is the current need to manually validate that every single reference type parameter is not null at runtime. This latest proposal seems to be de-emphasize that to the point where it is just mentioned as a possible "tweak" at the end. The way I understand it you would get warnings but zero guarantees. I believe I am far from alone in feeling that without fixing that this feature would be of limited value. Here is a vote for ensuring that runtime not-null guarantees are prioritized. It would probably remove something like 90% of the current parameter validation code that that C# developers write. I would suggest going a step further than the "!" syntax and supplying a compiler switch that would automatically generate such checks for ALL reference type parameters not explicitly allowed to be null by marking them with "?". This would be a huge time saver and if implemented in the compiler it should be possible to implement optimizations that would make the cost negligible. I would also suggest extending the "!" syntax to check for default(TValue) in struct parameters. So that you could, as one example, validate that a Guid is not Guid.Empty with just a single "!" character. |
|
@MikeyBurkman the var can infer the type to be string? with a known non-null value. This means the variable will pass all the tests and be able to be used as not-null until it is changed to the null value or a value not known to be not null. |
|
@Richiban you could only write that if T was either constrained to class or struct. |
|
@mattwar The issue with inferring it to the nullable value isn't that it's necessarily wrong, it's that it's extremely inconvenient, to the point that it becomes a nuisance. I don't recall seeing any suggestion about flow typing (where a variable's type can be narrowed by, for instance, a non-null assertion), so in order to pass that variable to a function not expecting a null, I'd either have to assign it some default value ( |
|
There have been multiple conversations on the Roslyn forums about this subject. The "nullability" isn't part of the type of the local, it's how it's used. So the following would produce zero warnings: string? s = "1234";
int len = s.Length; // the compiler knows that s is not null hereSo in most circumstances you wouldn't be forced to do anything special with the variable in order to placate the compiler, as long as the variable was definitely assigned to something that wasn't |
|
@mlidbom I'm against the Another question if |
@hades-incarnate I think your example is not correct, as it would have the same impact of spamming warnings just by using
So, the same language design choice you defend asserting it is worth billion dollars of investments in learning and preserving, for one of the biggest geniuses in computer science it actually caused billion of dollars in damage. Sometime is useful to remember we are just dwarfs standing on the shoulders of giants. |
|
Tony Hoare understates the cost just a little. I saw an article in CACM some years ago that estimated the economic cost of null pointer bugs as being in excess of a $US billion per year in the United States alone. This is why I'm personally very excited to see nullable reference types coming soon. (Apologies: I had a quick look to see if I could find the article to link, but wasn't able to do so.) |
|
I read the proposal and a few of the LDMs and I still don't understand how can I gradually adopt the feature without performing any migration? let's say I am upgrading my solution to C#8, I am using treat warnings as errors and I want to set the feature flag on, what I need to do in order for solu to compile? What I understand is if I turn the feature flag on,oblivious reference will become non-null references. am I correct? if so what happens with existing libraries and the .Net framework.? Where the feature flag is set? is it per solution? per project? |
Nothing, this feature doesn't produce errors, only warnings.
Oblivious references will become non-null references in your code. If you use a non-annotated library, that will stay oblivious, I believe.
There will definitely be a per-project flag and also some way to set it on a more gradual basis. I think it's not clear yet how exactly that will look, these LDM notes should contain the latest information. |
|
There are a number of ways of easing a large code base into this feature. Here is one sequence of steps:
|
|
What is the default value for a struct containing non-nullable reference types? Since all structs are zero-initialized will they be null? |
|
The fields would be null, that doesn't change. Accessing the non-null fields should result in a warning once roslyn#30731 is fixed. It seems to me that returning a partially-initialized |
|
I think it's a pity that the implementation of nullable reference types doesn't appear to permit extension methods, as is possible with value type's In contrast Kotlin, Rust and Swift have implemented value and reference nullable types in a similar fashion to C#
Workaround Is there any possibility that nullable reference types in future would support extension methods? |
What do you mean by that? For example, let's look at Swift's public static U? Map<T, U>(T? val, Func<T, U> func)
where T : class
where U : class
{
return val == null ? null : func(val);
}You can now have: string? s1 = "hello";
string? s2 = null;
string? s3 = s1.map(v => v + " world");
string? s4 = s2.map(v => v + " world");In this case, |
Quite right; I just tested that successfully a few minutes ago, and it worked as expected. |
|
Sure! |
|
From recent design notes:
I have an idea that could mitigate this pain point. Let's use the (in)famous public class Person
{
public string FirstName { get; set; }
public string? MiddleName { get; set; }
public string LastName { get; set; }
}Right now, enabling nullable will add two warnings that non-nullable properties But as the developer, you know that this type is only a DTO to negotiate between your code and a serialization framework, which the vast majority of the time will construct the object and fill in the properties via reflection prior to handing it back to you. My proposed solution is another attribute: [DTOType]
public class PersonThis attribute informs nullable flow analysis to supress the warnings about the un-initialized properties at the class definition, but when the properties are called from elsewhere, the nullable annotations are respected as normal. So what about the object initializers?The slightly rarer cases that you call var p = new Person
{
FirstName = "Joe"
}; //warning: Non-nullable property 'LastName' remains uninitialized.In addition, if during development it becomes apparent an additional non-nullable property has to be added to Thoughts? |
|
@Joe4evr
|
|
At least EFCore and Json.NET supports building a type through its constructor which means that the DTO could be made immutable which should resolve the issue. Though I understand that this isn't applicable to all serializers, and immutable DTO's can be a bit of a back pain sometimes, but with that it should be a non-issue. I don't understand why you should be able to not assign a non-nullable field though? Just make the property nullable instead of making a pinky-swear type. |
|
+1 to everything @GeirGrusom said and bravo on the term “pinky swear type.” I love it!
|
I don't know if you've noticed, but the entire implementation of Nullable Reference Types revolves around pinky-swearing what is and is not allowed to be Remember, this feature's mantra is "We can protect you from Murphy, not from Machiavelli." |
|
But this is trying to completely violate the point of the future to begin with. It will just punch another hole simply for convenience. |
|
The new attribute is an interesting proposal, but I agree with others who say “It will just punch another hole simply for convenience.” To go into your proposal more @Joe4evr: Wouldn’t it be better to enhance the serializer to suppress the warning rather than accidentally providing a way for non-serializer code to bypass the null safety? (Note: I might be misunderstanding your proposal)
I hear you, but I feel like the serializer should be capable of getting around this as opposed to introducing an unnecessary additional annotation/attribute. That’s at least my hope since most of the serialization scenarios already involve translating between a dynamic world (HTTP, JSON, or a file) into a static type. So in a sense, the serializer already knows how to ignore warnings. So if we keep that in mind, all you need to do to your Person example is to add a constructor that allows for developers to set the non-nullable fields. Side note: @Joe4evr it’s not a pinky swear type if you make something nullable. When you denote something as nullable you’re doing the opposite of making a promise— you’re telling the consumer to validate it themselves. In regards to your comment about “the entire implementation” I think what is wonderful about the 8 beta is that it allows you to check at the boundaries. This quote from a TypeScript best practice article (where nullable reference types has existed for years) really sums up the beauty perfectly. I can’t wait for C# to get there too:
|
This is because the warnings are in the completely wrong place. These types rarely have ctors or initializers because they're intended to be initialized by consuming code, and that's exactly where the enforcement should lie. Warn if the consumer doesn't initialize all of the non-nullable properties/fields before they allow the DTO to escape. I grant that this may require another attribute to annotate the types that should behave this way. // no warnings on this type
public class PersonDto {
public string FirstName { get; set; }
public string LastName { get; set; }
}
static void M() {
var dto1 = new PersonDto { FirstName = "Bill", LastName = "Microsoft" };
M2(dto1); // no warning
var dto2 = new PersonDto { FirstName = "Jeff" };
dto2.LastName = "Amazon";
M2(dto2); // no warning
var dto3 = new PersonDto { FirstName = "Tim" };
M2(dto3); // warning
dto3.LastName = "Apple";
M2(dto3); // no warning
}These types are incredibly common and aren't going to switch to a different paradigm. The same exact problem would apply to immutable DTOs with a mutable and initializable builder. The compiler needs to conform to how people write and use these types, otherwise the feature loses its value. |
|
I think this might be worth moving into a separate proposal. |
|
Created a seperate proposal: #2328 |
jcouv
changed the title
333fred
added
the
Implemented Needs ECMA Spec
and others
Note this includes the
objectconstraint:where T : object.LDM:
!)!, cast, impact of dereference on null-state, extension to nullable value types,class?,MaybeNull)?.,!on l-values, pure null tests)defaultloophole)MaybeNull)[DoesNotReturn])Task<T>variance, cast)T??)The text was updated successfully, but these errors were encountered: