null-forgiving operator is a mistake

[alexk8]

I propose a null-forgiving operator "!" to be null-unforgiving, null-check operator. What's the point of forgiving nulls instead of checking them? That's quite unsafe null-leakage. Consider the code:

void method1(string? str){
  method2(str!);  // <== NRE should be here
}
void method2(string str){
  DoLotsOfWork();
  str.Length // <== NRE is currently here
}

Another example:

class DTO{
  public object something {get;set;} = default!
}

I think that's also unacceptable and should throw exception. This should be something like this:

class DTO{
  [LateInitialization]
  public object something {get;set;}
}

Full discussion thread is in #2244, but I want to point out only one, single critical issue. (x!) should be replaced (by compiler) with (x != null ? x : throw NRE() ) to prevent massive null-leakage

[CyrusNajmabadi]

What's the point of forgiving nulls instead of checking them?

1. to prevent having to put checks in for when you know it's safe, but the compieler was unable to prove it was.
2. to make it much less painful to move an existing codebase forward.

You can always write an analyzer that disallows this construct and enable that on your code if you don't want to allow this to be written.

[terrajobst]

1. to prevent having to put checks in for when you know it's safe, but the compieler was unable to prove it was.

I think the proposal from @alexk8 is to allow the ! but making it emit a null check and throw NRE in case it's not-null at runtime. This would still address your case.

Personally, I could accept that but I think it wouldn't buy as much. In practice, when you actually have a null at runtime you're going to crash -- the question is now where. Most of the time that location is close to the bang operator, like in the OP's code where method2 would throw ANE (well should, but he isn't guarding that).

The downside of emitting the null check is code bloat which potentially doesn't play nicely with other JIT features, such as inlining, which to me outweighs the minor positive.

[alexk8]

1. to prevent having to put checks in for when you know it's safe, but the compieler was unable to prove it was.

How can man be sure if it's safe? If variable is declared and initialized within one method - compiler can prove, I suppose, otherwise neither man can, so, null-checking won't hurt much. Another example - default!. Are you sure it's safe?

2. to make it much less painful to move an existing codebase forward.

yes, it will be even more helpful

You can always write an analyzer that disallows this construct and enable that on your code if you don't want to allow this to be written.

No, I want to use it as a null-check. Can I turn a switch to make "!" null-unforgiving? But I also want to prevent null-leakage from libraries, not only my code

The downside of emitting the null check is code bloat which potentially doesn't play nicely with other JIT features, such as inlining, which to me outweighs the minor positive.

I'm not really aware of internals of JIT inlining, I admit it can probably "hurt" in some sutiations. I hope there willl be no bloat at all - I expect to see as little "!" as possible, and when I do - I want the compiler to assist with safity. I especially don't like default! statement which is recommended in some sutuations (DTOs). That allows to store nulls in non-nullables legally, and "!" operator is actually a backdoor for it.

[alexk8]

By the way, the most-used !. ( x!.Member) doesnt have to null-check anything, as long as exception will be thrown anyway (if null).

[khm1600]

If variable is declared and initialized within one method - compiler can prove, I suppose, otherwise neither man can, so, null-checking won't hurt much.

Bar? o = Foo();
if (NotNull(o))
{
    //man can prove o is not null here, compiler cannot
    //given a correctly implemented NotNull implemented before 8.0 shipped
}

[alexk8]

(NotNull(o))

In my opinion that's quite far-fetched example. Why would anyone rely on some NotNull implementation, that gives no guarantee. Why not to refactor it in correct way? Too many warnings? Disable/Ignore them for a while correct way:

if (!(o is null) && NotNull(o)) ...

[khm1600]

In my opinion that's quite far-fetched example. Why would anyone rely on some NotNull implementation, that gives no guarantee.

It can be Validate which happens to check for null. Also people rely on libraries not implemented by themselves a lot (.NET Framework, for example). Are you trying to say that everyone should implement everything they use?

[alexk8]

Are you trying to say that everyone should implement everything they use?

no, doublecheck or ignore warning (in this case) object can be checked against null quite easily, and other complex validation may be delegated to Validate

[CyrusNajmabadi]

In my opinion that's quite far-fetched example. Why would anyone rely on some NotNull implementation, that gives no guarantee.

One of the most popular .net APIs is string.IsNullOrEmpty. Are you saying that you would expect that even after calling that to validate arguments, the compiler would still warn people about using the string even if it had passed that check?

[khm1600]

Are you trying to say that everyone should implement everything they use?

no, doublecheck or ignore warning (in this case)

Why would you trust everything of a certain library but just don't trust its null checks? Unless the author is notorious for documenting null checks but not implementing them, maybe?

[CyrusNajmabadi]

How can man be sure if it's safe?

The same we we are sure of many other things. We have intelligence and inference systems that go beyond what a PL invariant system can generally express.

Why not to refactor it in correct way? Too many warnings? Disable/Ignore them for a while correct way: if (!(o is null) && NotNull(o)) ...

that's... awful. But if you like that, the solution is simple: code that way.

Nothing about this feature is stopping you from being able to do that. If you want stricter rules, you're welcome to write a tool that insists on those stricter rules. It won't even be that hard to write. Heck, i'm even happy to help you write it. It's really easy to encode these types of rules since they would be extremely conservative and would basically flip things around to only allow reads/writes in locations proven to be safe.

[CyrusNajmabadi]

no, doublecheck or ignore warning (in this case)

If i have 10,000 warnings, i'm not going to do anythign about it. And the 5 cases where there was something legitimate and worth fixing will be completely lost in that noise. If we make it so that people have to ignore the warnings all the time, it's going to make the feature non-adoptable by the community at large.

[alexk8]

• If you are intrested in code quality, you'll have to fix it someday
• I dont believe that somebody will have 10000 warnings of that type. maybe some dozens at max. usually people do not do like in your example, and if they do - they have a bad code. Please note I admit that I do not see the big picture, but that's my opinion.

[YairHalberstadt]

@Alexk8

If you are intrested in code quality, you'll have to fix it someday

I am extremely interested in code quality.

But I currently have a code base with over 1 million lines of code, and I guarantee that turning on NRTs will raise tens of thousands of issues.

The worst thing I can do, is to change my code whilst adding in the annotations, as that means that I'm taking tens of thousands of lines of debugged, working code, and changing them in non - semantically equavelent ways. Every single such change now has to be retested. I need some non-semantically changing way of telling the warnings to Shut Up For Now, whilst still allowing me to annotate my APIs.

Then when I refactor that part of the code base, I can remove the null forgiving operator.

[alexk8]

I agree that string.IsNullOrEmpty is used a lot. Probably it should be marked with a [ExtendedNullCheck] attribute, so compiler could use that knowledge

[YairHalberstadt]

For a real life example of this, see the discussion starting at this comment: https://github.com/dotnet/coreclr/pull/24258#discussion_r279261024. A change to fix nullability analysis caused a different exception to be thrown, which was deemed unacceptable.

[yaakov-h]

@khm1600 the compiler can prove that your object is not null based on the return value of NotNull(o).

Sharplab example here.

[alexk8]

But I currently have a code base with over 1 million lines of code, and I guarantee that turning on NRTs will raise tens of thousands of issues.

you're talking about entire NTR feature, I understand that's a lot of warnings, but I was talking there about small thing like "not-null proving". Concerning the "null-UNforgiving operator" - it will not break your code at all - that's only a null-check instead of null-leak probability

For a real life example of this, see the discussion starting at ...

I think it'll be quite difficult to dive into the middle of conversation, but I'll try, thanks

[YairHalberstadt]

@Concerning the "null-UNforgiving operator" - it will not break your code at all - that's only a null-check instead of null-leak probability

A null check can definitely break my code.

I may assign something nullable to a non-nullable property in a constructor, and then check if it's null later and set it to something non null if it is.

Eg.

public class A
{
    public string B {get;}
    public A()
    {
        B = TryGetString()!;
        B ??= "";
    }
}

Making the null forgiving operator here make a check will break my working code.

Now of course, I could easily refactor this to fix this. But that's exactly my point. I do not want to make any changes right now. That means I have to test everything all over again. I just want to add in a ! which I know has no runtime meaning, and will finish this up when I come back to it.

[khm1600]

@yaakov-h oh, I didn't know that.

[alexk8]

@YairHalberstadt Imho, You've taken up new Beta-feature in wrong way! Good way:

public class A
{
    public string B {get;}
    public A()
    {
        B = TryGetString() ?? "";
    }
}

all code that began to use "!" in a wrong way will break, yep

[alexk8]

that's what I was talking about. the "!" operator introduced it's misusage. luckily it's not in release yet and may be changed

[yaakov-h]

@alexk8 congratulations, you've just omitted a call to set_B and goodness knows what side effects that will bring.

[YairHalberstadt]

@Alexk8 That's exactly my point. Of course I will clear this up later. But right now I've got a thousand nullable warnings and just want to annotate my code without risking any more change than absolutely necessary. It would be terrible practice to willy nilly change things unless I actually have time to do so properly, and test every change.

[alexk8]

@alexk8 congratulations, you've just omitted a call to set_B and goodness knows what side effects that will bring.

if B can have null then what the heck it is not-nullable? use it like public string B? {get;} then

[alexk8]

@YairHalberstadt please don't start porting your releases until c# 8 is released today we can only experement with that) however, I don't believe they will change it, though I hope

[john-h-k]

I want to tell the compiler I know something isn't null. I don't want the compiler to emit a random, unnecessary null check. What do I do?

[alexk8]

I want to tell the compiler I know something isn't null. I don't want the compiler to emit a random, unnecessary null check. What do I do?

Well, I do not have answer for that question right now... Good if you're 100% know something isn't null,
But I see lots of people using this for other purpose - just to make compiler shut up. default! , null! - lots of them. 'Hey, compiler, eat my null, I'll initialize it later.'

In your case extra null-check probably won't hurt, unless it's a huge loop, and I suspect there is not many such cases, none, if everything's done right (get NRTs, use like NRTs) Examples are welcome

[yaakov-h]

and I suspect there is not many such cases

A cursory search of my company codebase for Contract.Assume(foo != null) suggests very strongly otherwise.

[alexk8]

Sorry, I do not understand. Once you added #nullable enable, all foos supposed to be non-nullable, no "!" needed at all. Please explain what did you mean

[CyrusNajmabadi]

If you are intrested in code quality, you'll have to fix it someday

That's not true at all. You can literally have warnings/analyzers for anything, and they can be far too aggressive and unnecessary. It may be simple to prove things in your code that can't be expressed in the type system. Insisting that there's a code quality problem because you have warnings being issued from a weak source of truth is invalid.

Furthermore, things can and will change over time. Take a look at TypeScript and how they gently add new 'strict' features as time goes on to gently help move codebases forward instead of hampering them with far too many issues to get through at once.

I dont believe that somebody will have 10000 warnings of that type.

This is literally the most common null check that is seen. Someone checks a variable in some location for null, they then use it. In nearly 100% of cases this is a fine pattern.

maybe some dozens at max. usually people do not do like in your example, and if they do - they have a bad code.

There is nothing bad about this code. In many domains that code will be correct 100% of the time. And, even in hte domains where it isn't, it's likely to only be incorrect an incredibly tiny amount of the time.

Are you aware of the concept of 'alarm fatigue'? It's a very real thin and it's something that needs to be avoided here.

[CyrusNajmabadi]

Imho, You've taken up new Beta-feature in wrong way!

No he hasn't. The point is to provide people a gentle way to move forward, allowing them to stop at points they can accept.

Good way:

You're now forcing him to fixup all these locations. Not only is that extremely painful and costly, it's also very risky because those refactorings may change program behavior.

@alexk8 you are insisting that every team that wants to adopt this be able to absorb and accept these costs and that because you are willing to everyone else must as well. that's now how this works. There are teams out there that will likely take a decade to fully move over to something like NRT. If the up front cost is this extreme, it simply will not happen, and we'll end up with multiple C# dialects (essentially different languages). That is anathema to the goals of the C# team. So a great effort has been put into ensuring that this feature can be adopted in a gentle manner so the ecosystem can eventually move over effectively.

[CyrusNajmabadi]

And, @alexk8 i keep pointing out that Roslyn was designed to give you the level of analysis and restrictions that you say you want. You can write analyzers here. You can say "the language is too loose, i'm going to put in the rules that make things as strict as i want". I even recommend that you do this. I recommend you try out this exercise to see what sort of impact it has on a codebase. Heck, try it out on your code, then try it out on some large github projects (like Roslyn itself). It would be very valuable for helping you understand the issue and for getting you over this bit:

Please note I admit that I do not see the big picture, but that's my opinion.

You keep explaining your perspective from your own project. And trust me, everyone here understands your POV. The problem is not htat your POV is wrong for your project, it's that its not acceptable for the vast gamut of teams and projects that we know about that we do want adopting this feature.

[john-h-k]

Well, I do not have answer for that question right now

Then find one, because this proposal is flawed in the sense I don't want a bunch of random null checks for no reason

[alexk8]

Then find one, because this proposal is flawed in the sense I don't want a bunch of random null checks for no reason

My answer was that you probably won't need that. And you aren't giving any examples. Here is a solution:

void method(T? nullable){
  T nonnullable1 = nullable!;  // <-- for most use-cases I want null-ckecking, so syntax is simpler
  T nonnullable2 = (T)nullable;  // <-- for use cases where you want to tell the compiler you know more
}

[alexk8]

@CyrusNajmabadi You are talkink about warnings... That's absolutely same amount of warnings. No changes. Key point is a behavior: null-leaking or null-ckecking, which is better for code-quality?

Let's split this concern into two parts: 1) in statements where you know nulls will not emerge - Ok I aggree that null-forgiving is acceptable. 2) its misusage like default!. Do yo think that's a good practice?

Next, now or in the future, we may want to turn on strict mode and all this null! will blow up. Wise.

[alexk8]

However, it's too late, the train is off. May I propose new !! null-unforgiving operator in that case?

[CyrusNajmabadi]

You are talkink about warnings... That's absolutely same amount of warnings. No changes.

what do you mean? in the current world you get less warnings. In the world you are describing htat you want, there would be many more warnings.

[CyrusNajmabadi]

its misusage like default!. Do yo think that's a good practice?

I think good practice will be to adopt nullable slowly over time in a low risk manner. To do that, there will occasionally be places where it is convenient and expedient to nip things in the bud. To that end, i think ! serves its purpose very well.

I know in TypeScript it's been a valuable and useful tool for many sorts of situations where you want to adopt nullable, and can do it in about 95% in cases, but don't have the resources to do the large refactoring to make it totally work properly in 100% of all cases.

I'd rather people be able to get to a place that is 95% better easily, and then allow them to take on extra burden than to deliver a feature that makes it much more costly to even get to 5% better.

[alexk8]

what do you mean? in the current world you get less warnings. In the world you are describing htat you want, there would be many more warnings.

Please explain what world have I described, in your vision. Literally I said that we should only change ! behavior - You turn on #nullable, see your warnings, put ! where you want. The only different thing - if you were wrong about 'I know this wont be null', null will be captured in runtime before it's leaked deep down and blown up somewhere else

[alexk8]

But the most important part of it is not guarding your competence, no. The bigger problem is that current ! operator allows its usage with an explicit and implicit nulls just to hide warning, it's used quite a lot and i think that's a real mistake

[CyrusNajmabadi]

You turn on #nullable, see your warnings, put ! where you want. The only different thing - if you were wrong about 'I know this wont be null', null will be captured in runtime before it's leaked deep down and blown up somewhere else

that's not waht i want though. I use ! when i know it's null, and i know it's ok to suppress the warning because i know it won't blow up later. It's a stopgap. You've changed by stopgap into a crash. That's a non-starter.

and i think that's a real mistake

So write an analyzer.

This is honestly going in circles. You're not adding any new information. You just keep saying that you don't like this. I get that you don't like it. So use the existing mechanisms that Roslyn has to address this. Write an analyzer that disallows you from using ! in these situations. Now you don't have to use it.

[CyrusNajmabadi]

Literally I said that we should only change ! behavior

Changing ! behavior in this way is not acceptable. You've now violated the goals mentioned in this post here: https://github.com/dotnet/csharplang/issues/2477#issuecomment-488080560

[CyrusNajmabadi]

May I propose new !! null-unforgiving operator in that case?

Just write an extension method that does that for you:

expr.ThrowIfNull();

...

public static void ThrowIfNull<T>(this T t) where T : class
{
    if (t == null) throw new WhateverYouWantException();
}

[CyrusNajmabadi]

@alexk8 you keep asking for things to be more restrictive and for people to have to be more explicit with their code to prove it is truly null safe. As i've mentioned probably 10 times now: you can get this. Roslyn was designed for this very purpose. People already use thi today to write analyzer that check how different APIs are being used and to enforce that coding patterns around those APIs are proper.

This would be the same, jsut at a more general level since it would not be API specific, but how your code worked with any API. There are already many analyzers for this sort of thing today that are written to look for defects the language doesn't say anything about but the analyzer author was important enough to invest in.

What i do not get is why you keep ignoring this and are unwilling to just use hte existing system that has been available for years now to get the behavior around nulls that you want. If you want this sort of uber strict approach, go nuts and have it. Nothing is stopping you.

[alexk8]

that's not waht i want though. I use ! when i know it's null, and i know it's ok to suppress the warning because i know it won't blow up later. It's a stopgap. You've changed by stopgap into a crash. That's a non-starter.

I honestly do not understand that behaviour. You get a warning, go there to fix it somehow, but instead of real fixing just put there a plug

expr.ThrowIfNull()

actually I had similar one, and more smart and handy ones I want just syntactic sugar in that case. In more general case I dont want people use ! in a wrong way

[CyrusNajmabadi]

I honestly do not understand that behaviour.

Yes, and that's a deep problem. You need to understand, otherwise you're going to be blocked here a long time.

You get a warning, go there to fix it somehow,

Going and fixing is costly. It can be risky. There may be thousands or millions of these.

but instead of real fixing just put there a plug

Yes. As i said, sometimes a real development team needs a way to nip something in the bud.

In more general case I dont want people use ! in a wrong way

THEN WRITE AN ANALYZER THAT DOES THAT.

I'm putting that in all caps since you have ignored this over and over and over again. I honestly don't know how to get this information to you any other way.

[alexk8]

Yes, and that's a deep problem. You need to understand, otherwise you're going to be blocked here a long time.

Ok, I'll try to understand that.

I'm putting that in all caps since you have ignored this over and over and over again. I honestly don't know how to get this information to you any other way.

I understood your opinion quite long ago, but you the only one who constanlty tries to convince me with the same argumens. People sometimes have different opinions. If you have an authority, say that 'We wont do that, sorry' that would be enough for me

[yaakov-h]

I understood your opinion quite long ago, but you the only one who constanlty tries to convince me with the same argumens.

The rest of us likely tuned out long ago, because you keep repeating yourself without addressing any of the counter-claims made.

If the language permits something and you don't want that to happen in your codebase, then write an analyzer.

The dammit operator has been designed in C# to silence the compiler warnings, not explode your program. Is it extremely unlikely that this will change, particularly this late in the game.

[CyrusNajmabadi]

I understood your opinion quite long ago,

Then why are you still arguing? You want a way to not allow the behavior which you view as unsafe. Such a mechanism exists. I do not see why you will not use it.