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: Poisonous Null-checking #5961

Closed
alrz opened this issue Oct 14, 2015 · 43 comments
Closed

Proposal: Poisonous Null-checking #5961

alrz opened this issue Oct 14, 2015 · 43 comments
Labels
Area-Language Design Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.

Comments

@alrz
Copy link
Contributor

alrz commented Oct 14, 2015

With a lot of null-checking features introduced in C# language up to this version (and more on the way), there is some cases that still need for null-checking (assuming that it is a valid value so it's not addressed by #5032).

For example value in F(value); can be nullbut F can't be called with a null (in case of a non-nullable reference type), or shouldn't (throwing exceptions). So you need to check for null before calling it like

if(value != null) F(value);

In cases like this, there should be a feature in which you check for a null on an expression and if it's a null make it a poison value for the rest of the expression. e.g.

F(value?);

so F wouldn't be called if value is null.

If F is not a void-returning function, then the whole expression would be null.

object result = F(value?) ?? defaultValue;
// instead of
object result = value != null ? F(value) : defaultValue;

bool result = (foo.Bar != baz?) == true;
// instead of
bool result = baz != null ? foo.Bar != baz : false;

bool? result = F(Foo.Bar?.Baz?);
// instead of
bool? result = Foo.Bar?.Baz != null ? F(Foo.Bar.Baz) : null;

You can see how this would be useful.

@svick
Copy link
Contributor

svick commented Oct 14, 2015

For something that can completely change what a line of code does, I think this syntax is too subtle.

If I see F(expr1, expr2, …), I expect that F will be called, no matter what expressions expri are (with the exception of exceptions).

@HaloFour
Copy link

I agree. All of the other null-coalescing behavior starts from a root expression and collapses from there. This wants to do so from a child expression. But given how a function may accept any number of nullable arguments and any combination of them may be a reason to avoid the function call I don't know of an obvious shorthand syntax. What about the case of nested function calls?

@alrz
Copy link
Contributor Author

alrz commented Oct 14, 2015

@svick That's actually the same subtle thing that cause a line of code fails to do what it supposed to do. so you can just be sure this way that it won't. Although, I just proposed a syntax consistent with other null-checking operators so they can be used together as you can see in my examples. If you use a keyword, for this, that would not look like a null-checked statement anymore.

@HaloFour That's the point. A single null can poison all the expression. That's exactly what we are avoiding. Which is more readable to you?

F(arg1?, arg2?, arg3?, arg4?);

if( arg1 != null && arg2 != null && arg3 != null && arg4 != null )
    F(arg1, arg2, arg3, arg4);

By the way, in my last example, properties will evaluate just one time. Since you haven't args right where you need them like the code above.

@HaloFour
Copy link

@alrz Honestly, neither. The ?s blend right into the statement and if there are quite a few arguments could be entirely missed (or accidentally missed). I'd rather see syntax adorning the call to F() itself.

@alrz
Copy link
Contributor Author

alrz commented Oct 14, 2015

@HaloFour Honestly, that wouldn't happen in real code but in some cases, for one or two variable, null checks will cause to lose focus on doing the actual work and make the code mostly irrelevant. Specifically if variables have long names for readability. This syntax follows the same motivations as ?? and ?. operators. You can find a lot of examples of this in the roslyn code itself that would take advantage from this syntax.

@alrz
Copy link
Contributor Author

alrz commented Oct 14, 2015

I don't see why ?. returns null instead of default(T) when it fails.

@HaloFour
Copy link

@alrz Because that is the purpose of the null propagation operator. It doesn't make sense to apply to non-null operations. If you want the end result to be something other than null that's why the null coalescing operator ?? exists.

@HaloFour
Copy link

At best I could see some kind of syntax like this working if #5445 gets off of the ground (oh dear IPU I hope not). Then it at least would make sense to have a second forwarding operator like ?> which would function like the null-propagation operator.

bool result = Foo.Bar?.Baz ?> F ?? false;

@alrz
Copy link
Contributor Author

alrz commented Oct 14, 2015

@HaloFour returning a default(T) wouldn't change that behavior. If the type of rhs was nullable you would get your lovely null and then you can use null coalescing operator. By returning default(T) where T is the type of the rhs, for example argument.Type != null && argument.Type.IsRestrictedType() could be written as argument.Type?.IsRestrictedType() without creating a nullable bool.

btw, I liked that ?> syntax (without ?? false part though. because the way you wrote it, would result in a bool? not bool). What is wrong with #5445 then?

@HaloFour
Copy link

@alrz The ?. operator was designed very explicitly to bail with a nullable result. Returning bool? is appropriate, even for argument.Type?.IsRestrictedType().

Forward operators provide absolutely no functionality or even shorthand compared to what we have today. They're only really useful for very specific chains of functions. Once you have multiple parameters or void methods they start to break down and you have to fall back to normal function calls. The C# language shouldn't be a dumping ground for every possible syntax form ever to exist in every other language.

@orthoxerox
Copy link
Contributor

@HaloFour no it shouldn't, that's what Scala is for ;)

However, function piping is important for objectless programming style.

@alrz Am I correct that Bar(foo?) is equivalent to (foo!=null)?Bar(foo!):foo?

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@orthoxerox what is this foo!?

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour Re "Returning bool? is appropriate..."
I see, that is kind of a "monadic null checking" but since we have a special operator just for this purpose and C# is not that functional I think it would be more useful with default(T).

@orthoxerox
Copy link
Contributor

@alrz If I remember correctly, that's the "cast to non-nullable or throw" operator from the nullability tracking proposal.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@orthoxerox I don't see such a thing in #5032. Anyway, this won't throw any exception, because if foo was null, Bar won't be called at all.

@orthoxerox
Copy link
Contributor

@alrz it's in #5033

@HaloFour
Copy link

@orthoxerox No kidding. Scala is about as fun to read as an ASCII sneeze, and don't get me started on Java interoperability. But function piping avoids no more objects than nested function calls and C-family languages have been fine with that for decades. Or we have extension methods which make rewriting that as fluent quite easy.

Your assumption is exactly why I find this proposal confusing. You don't decorate the method call, but it affects the method call.

bool result = Bar(foo?);
// equivalent to
bool result = (foo != null ? Bar(foo) : default(bool));

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour With #5444 you can do more with forward pipe, for example

void F(Foo foo) => foo :> Foo::InstanceMethod :> Console.WriteLine(format: "Result: {0}");

It's not limited to nested, static or single parameter functions. It's all about order of execution and code style. Whatever you prefer.

But about confusion you referred, yes I assumed that the operator ? can affect the expression tree up to some point. which might be not a good idea. I suggest a checked-like keyword which explicitly specifies the affecting expression/statement.

bool result = checked(Bar(foo?));

Obviously, some other keyword. But you've got the idea.

@HaloFour
Copy link

@alrz It is exactly the same as nested function calls, except that sourcing multiple parameters from expressions is messier. And your proposal for supporting multiple parameters separately from using tuples by using named arguments is only more confusing since it blends the two forms.

There are dozens of languages targeting the CLR. If my preference is for COBOL-like syntax, I don't expect C# syntax to become COBOL.

But anywho, that horse is beaten and I really don't have constructive comments regarding forwarding, so I'll leave it to the language design team to consider whether it achieves the 200+ points necessary for consideration.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour I'm not saying that it should be considered exactly the same way that I proposed, but it will be a "forward pipe operator" anyway. I'll be happy to discuss this with you further over #5445 since the subject is irrelevant to this title.

@bbarry
Copy link

bbarry commented Oct 15, 2015

assuming some sort of null checked operator (foo! for example, #5033)

bool result = Bar(foo?);

should be the same as

bool result = foo != null ? Bar(foo!) : default(bool);

aside from the idea that the former is thread safe and the latter is not (foo could potentially be changed between the conditional evaluation and the method call).


This feels like postfix conditions in perl except they are potentially even more difficult to identify. 👎

? as an operator to do this is potentially confusing because it is already part of the ternary operator. While syntax there is not ambiguous, it could be difficult for the programmer.

If the forward pipe operator comes in (#5445) I would prefer also having a conditional forward pipe operator and the syntax ?> would be ambiguous here:

var x = y?>z;

(you would need to do semantic analysis to determine what the syntax tree for that statement is; this is even worse with extension operators: #4945)

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@bbarry With the proposed syntax you can do that: var x = y? :> z; which is equivalent to var x = z(y?); which is equivalent to var x = y != null ? z(y) : default(T); where T is the type that function z returns. The operator ?> is not proposed.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

The operator ? makes the existing ?. and ?[ operators ambiguous and the reason is, with introducing the former, there is no need for the latter.

// null-checked member access
var result = obj    ?.  Member;
// null-checked expression
var result = obj?      .Member;

Actually what is proposed here is a generalized version of null-conditional operators.

So there are two options left:

  • Remove the previous ?. or ?[ operators and introduce the generalized version ?. Although, for backward compatibility this needs to return null in case of failure not default(T).
  • Use a new operator like !.

Introducing check keyword might help with this ambiguity.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@gafter I wonder why the team didn't consider this at first place and specialized it just for member access and indexers?

@HaloFour
Copy link

That makes no sense. Null propagation doesn't work without member access of some sort. It's how that works in the variety of languages that already support it like Swift and Oxygene. What is var result = value?; even supposed to mean? You can't get nuller than null. And suggesting that it should return some kind of default makes just as little sense especially given that the default of any reference type is already null.

The ! postfix operator is already being proposed as an explicit unwrap operator for nullable references, e.g. string? foo = ...; int length = foo!.Length;

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour Yes that doesn't make sense. Because you actually do null-checks when you want to use a value. whether in a member access or function call or whatever. Returning default(T) makes sense when you want to return a value type. while it doesn't change anything when working with a reference type. btw, the reason for existing a default(T) is this little difference between value and reference types so you can safely use a default(T) in a generic method, no matter if class or struct constraints are applied.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour your example makes sense when "referential transparency" applies, e.g. the assignment let result = value? in ...; (#3718)

@HaloFour
Copy link

Except that null propagation propagates Nullable<T> in place of an expression that would result in T, so default(T) doesn't apply and doesn't make sense. It also is not designed to propagate up to parent nodes of the expression, only to children. It's right-associative for a reason (which was not the initial design, btw). The discussion as to the reason for the null propagation operator in C# is still available across quite a few posts on CodePlex. As mentioned, this operator exists in this form in other languages, it wasn't invented by the C# team.

@HaloFour
Copy link

#3718 is just a form of declaration expressions with a more complicated syntax. It doesn't make sense in that context either as result would still be assigned some value (null) and the remainder of the expression executed. Unless you're claiming that it wouldn't be, which would be a different proposal altogether, and even more unintuitive.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

It doesn't make sense in that context either as result would still be assigned some value (null)

Since let-in syntax followed by an expression, if value was null then the expression would not be evaluated and default(T) shall return where T is the type of that expression. I don't see why it would be a different proposal since I do proposed using ? in an expression too. (Note that the whole let-in returns ergo it is also an expression.)

Except that null propagation propagates Nullable<T>

Exactly, why null propagation makes it a special case for value types while it doesn't make sense to continue the propagation after yielding a Nullable<T>? e.g. foo?.BooleanProperty?. /* what could you possibly insert in here */ if you want to say ?? defaultValue so why it doesn't return default(T) in the first place?

@HaloFour
Copy link

@alrz Which means that you're amending the purpose of that proposal.

Null propagation propagates nulls. The null of a value type is default(T?). This is on purpose and by design. Without that you couldn't use that coalescing operator in order to provide your own default value, which would make s?.Length ?? -1 or o?.IsSomething ?? true impossible.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

s?.Length ?? -1 or o?.IsSomething ?? true

I think wanting a default(T) (like false) from that expression is more common. You can see countless of times that this is happened in roslyn code itself, and they had to use the old fashioned way of if(obj != null && obj.IsSomething ) to not allocate an unnecessary Nullable<bool> while this is exactly the main purpose of the ?. operator.

@HaloFour
Copy link

@alrz

I think wanting a default(T) (like false) from that expression is more common.

Being common isn't relevant, providing the choice to the developer is, and that is only possible if the expression returns null. Either way, that ship has sailed with C# 6.0 and the likelihood of the C# team considering a breaking change to it is a complete non-starter.

This is the exact same behavior as with Apple Swift. Optional chaining always results in an Optional<T>. residence?.numberOfRooms results in an Int? even if numberOfRooms is defined as an Int.

to not allocate an unnecessary Nullable<bool>

Nullable<T> is a struct, there is no allocation.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour unnecessary cast I mean.

Being common isn't relevant, providing the choice to the developer is

This is exactly what is recommended to avoid in any production design. It should make sense not just providing a choice for the user. The point is, you can live without ?. but it makes life easier because it's very common that you want to access an object's member which might be null, you know.

@HaloFour
Copy link

unnecessary cast I mean.

Casts are cheap, as is the overhead of unwrapping a Nullable<T>.

This is exactly what is recommended to avoid in any production design. It should make sense not just providing a choice for the user.

Wait, are you seriously arguing that a "production design" programming language should eliminate choices for the developer?

The point is, you can live without ?. but it makes life easier because it's very common that you want to access an object's member which might be null, you know.

Which is exactly why ?. does exist, and why it works so well with the ?? operator, despite your objections. It's behavior is not going to change.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

Casts are cheap, as is the overhead of unwrapping a Nullable.

Yes they are, but the team still prefer to not use it for a reason, right?

programming language should eliminate choices for the developer?

Yes, check out the closed issues.

Anyway, so you are against the first bullet too? Regarding backward compatibility.

@HaloFour
Copy link

Yes they are, but the team still prefer to not use it for a reason, right?

You'd have to ask them. My guess would be that C# 6.0 features are very underrepresented in the Roslyn codebase, primarily because the majority of that code was written before those features existed and they didn't bother rewriting it.

Yes, check out the closed issues.

There is a big difference between deciding to not adopt unnecessary features or syntax which does not fit the language vs. designing those features with a specific locked down and inflexible behavior.

Anyway, so you are against the first bullet too? Regarding backward compatibility.

Your first bullet involves removing an existing feature. Considering any new feature starts with a score of -100 I'd think that a feature predicated on changing the nature of an existing feature would immediately incur quite the penalty. You'd have to demonstrate an extraordinary amount of value in this proposal in order to even break even.

@alrz
Copy link
Contributor Author

alrz commented Oct 15, 2015

@HaloFour It doesn't change the nature of the existing operators, it just generalize them by segregating them into one new operator and the existing one. Actually, if that happens it would be a smart design with backward compatibility in mind. That's why introducing check keyword wouldn't help. You're saying that it's impossible or it's not worth the effort?

@gafter
Copy link
Member

gafter commented Oct 16, 2015

@gafter I wonder why the team didn't consider this at first place and specialized it just for member access and indexers?

I wonder why @alrz didn't wonder this earlier.

@alrz
Copy link
Contributor Author

alrz commented Oct 16, 2015

@gafter you got me there 😄. but I think this is still possible without changing the existing behavior.

@gafter gafter added the Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. label Oct 21, 2015
@alrz
Copy link
Contributor Author

alrz commented Oct 21, 2015

@gafter "won't fix" as it's not possible or it's not worth the effort? I was really looking forward for this, it would simplify a geat deal of repetitive code and save some property/method calls along the way (my third example).

@gafter
Copy link
Member

gafter commented Oct 21, 2015

@alrz I'd rather crush your hopes now than leave you hanging forever 😉

I have a lot of concerns about this - how far up the expression does this operator infect things; how does it affect the type sysem; how does it affect readability; what syntactic issues arise from a postfix ? operator? It feels to me like the language would be worse with this addition rather than better. Every expression form would have to have specification text in it explaining what would happen if there is "poison" involved.

I can't imagine anyone on the language design team championing this proposal, and that is what it would take for this (or any other proposal) to happen.

@jrmoreno1
Copy link
Contributor

The way to do this now is to wrap it in a try/catch and throw using the ref?.property ?? throw syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.
Projects
None yet
Development

No branches or pull requests

8 participants