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

Null-conditional assignment #6045

Open
2 of 4 tasks
RikkiGibson opened this issue Apr 21, 2022 · 29 comments
Open
2 of 4 tasks

Null-conditional assignment #6045

RikkiGibson opened this issue Apr 21, 2022 · 29 comments

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Apr 21, 2022

Summary

Permits assignment to occur conditionally within a a?.b or a?[b] expression.

using System;

class C
{
    public object obj;
}

void M(C? c)
{
    c?.obj = new object();
}
using System;

class C
{
    public event Action E;
}

void M(C? c)
{
    c?.E += () => { Console.WriteLine("handled event E"); };
}
void M(object[]? arr)
{
    arr?[42] = new object();
}

The full proposal for this feature has moved to https://github.com/dotnet/csharplang/blob/main/proposals/null-conditional-assignment.md.

@RikkiGibson RikkiGibson added the Proposal Question Question to be discussed in LDM related to a proposal label Apr 21, 2022
@CyrusNajmabadi
Copy link
Member

We could instead make the ?. syntactically a child of the =. This makes it so any handling of = expressions needs to become aware of the conditionality of the right side in the presence of ?. on the left. It also makes it so the structure of the syntax doesn't correspond as strongly to the semantics.

i would not do this :) Let's be consistent with the way we already do conditionals today.

On the IDE, we'll just need to update some of our conditional helpers we have today, and we'll have to audit all code that handles assignments. Seems doable.

@quinmars
Copy link

I'd expect

M(a?.b?.c = d);

be equivalent to:

M(a is null
    ? d
    : (a.b is null
        ? d
        : (a.b.c = d));

so that for any expression, be it a.b or a?.b:

var z = (any_expression = d);

z would always be of the type of d and had the value of d. I know that would go against the lazy evaluation. And probably that is more important in real code, inparticular since the value of an assignment is rarely used. But for a carefree reader, it could cause surprise.

@CyrusNajmabadi
Copy link
Member

I would expect that if 'a' was null that no further work was done. That's the prime use case as otherwise we'll execute expensive work that will be thrown away. Having this work differently fro an argument vs an expression-statement would just be enormously weird at that point. If someone ends up writing M(a?.b?.c = d); my expectation woudl be that they're thinking htey can get null as a being null would mean no work should be done at all.

I would also be entirely ok with the expression form of this being disallowed, and only allowing the statement form.

@RikkiGibson
Copy link
Contributor Author

var z = (any_expression = d);

z would always be of the type of d and had the value of d.

d is converted to the type of any_expression here, and the result type of the = expression is the same as the type of any_expression.

If we were to adopt the semantics you described @quinmars, then in a?.b?.c = d, we would probably want to convert d to the type of a.b.c and use that as the result type of the expression.

a?.SetX(M()) has semantics where M() is only evaluated if a is not null. I think if anyone ended up moving from that form to a?.X = M() due to improved ergonomics here, they'd be disappointed if it ended up having different semantics.

@quinmars
Copy link

Nevermind, you are both correct. The lazy evaluation is a very important part of the feature and shouldn't be sacrified for my first impression when seeing this code. I find the symmetry argument (a?.SetX(M()) vs. a?.X = M()) most compelling. Fortunately, the type system will catch wrong assumptions (z has the same type as d) easily - at least for value types. Anyway, the expression form of the assignment is seldom used.

@jnm2
Copy link
Contributor

jnm2 commented Apr 21, 2022

I would also be entirely ok with the expression form of this being disallowed, and only allowing the statement form.

This has consequences for lambda return type inference, so it would leave a weird back-compat wart if this restriction was shipped and then loosened later.

@CyrusNajmabadi
Copy link
Member

Let me rephrase: i would prefer to disallow one of the forms over having the two forms have different semantics :)

If hte two forms have the same semantics, then there's no problem and i wouldn't disallow either of them.

@jnm2
Copy link
Contributor

jnm2 commented Apr 21, 2022

I agree, having the expression mean something different when directly a statement would be frightening.

I also initially expected a = b?.c = d to mean temp = d; b?.c = temp; a = temp;, but that's incompatible with my other strong expectation that d is not evaluated when b is null. So the first expectation has been revised.

@333fred 333fred added Proposal Proposal champion and removed Proposal Question Question to be discussed in LDM related to a proposal labels Apr 27, 2022
@333fred 333fred added this to the Working Set milestone Apr 27, 2022
@michael-hawker
Copy link

Awesome to see this moving forward! 🎉 Just wanted to bubble up our use case from the Windows Community Toolkit where we have to do event hooks all the time (from this original comment here):

We do this all the time in the toolkit for templated controls, where we need to register event handlers:

            if (_tabItemsPresenter != null)
            {
                _tabItemsPresenter.SizeChanged -= TabView_SizeChanged;
            }

            if (_tabScroller != null)
            {
                _tabScroller.Loaded -= ScrollViewer_Loaded;
            }

            _tabContentPresenter = GetTemplateChild(TabContentPresenterName) as ContentPresenter;
            _tabViewContainer = GetTemplateChild(TabViewContainerName) as Grid;
            _tabItemsPresenter = GetTemplateChild(TabsItemsPresenterName) as ItemsPresenter;
            _tabScroller = GetTemplateChild(TabsScrollViewerName) as ScrollViewer;

            if (_tabItemsPresenter != null)
            {
                _tabItemsPresenter.SizeChanged += TabView_SizeChanged;
            }

            if (_tabScroller != null)
            {
                _tabScroller.Loaded += ScrollViewer_Loaded;
            }

It'd be nice to condense this down to this:

            _tabItemsPresenter?.SizeChanged -= TabView_SizeChanged;
            _tabScroller?.Loaded -= ScrollViewer_Loaded;

            _tabContentPresenter = GetTemplateChild(TabContentPresenterName) as ContentPresenter;
            _tabViewContainer = GetTemplateChild(TabViewContainerName) as Grid;
            _tabItemsPresenter = GetTemplateChild(TabsItemsPresenterName) as ItemsPresenter;
            _tabScroller = GetTemplateChild(TabsScrollViewerName) as ScrollViewer;

            _tabItemsPresenter?.SizeChanged += TabView_SizeChanged;
            _tabScroller?.Loaded += ScrollViewer_Loaded;

@Duranom
Copy link

Duranom commented Aug 28, 2022

Question concerning the P?.A = B; to be equivalent to if (P is not null) { P.A = B; }.
Is this the case for class types and structs? Or only for structs?
Because for classes I would expect P?.A = B; to be equivalent to if (P is {} __p) { __p.A = B; }

For structs it also feels a bit weird to see P?.A = B; being equivalent to if (P is not null) { P.A = B; }.

Reason I ask is that it is easy enough to get race conditions with this, especially with UI or those that handle parallelism and it can easily assign incorrectly in cases when not aware.

@CyrusNajmabadi
Copy link
Member

As the OP says:

P?.A = B is equivalent to if (P is not null) P.A = B;, except that P is only evaluated once.

@Duranom
Copy link

Duranom commented Aug 30, 2022

My bad, the repeated parts later in example on put on wrong footing, as it feels far from equivalent.
But still wondering how it will go with structs, are those magically going to be referenced? Or will there be a need for an analyzer to warn if the struct is not done by reference?

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Aug 30, 2022

@RikkiGibson for thoughts on nullable structs.

@RikkiGibson
Copy link
Contributor Author

RikkiGibson commented Aug 30, 2022

I think we probably have to block assignments to members of nullable structs for the same reason we block the following: SharpLab

public struct S
{
    public string Prop { get; set; }
}

public class C {
    public void M(S? s) {
        s.Value.Prop = "a"; // error CS1612: Cannot modify the return value of 'S?.Value' because it is not a variable
    }
}

s?.Prop = "a" would be the same error.

@CaseyHofland
Copy link

CaseyHofland commented Sep 9, 2022

s?.Prop = "a" would be the same error.

Just as well that's easy enough to include. A simple check to see if you're dealing with a Nullable<> and the condition evaluates to:

s?.Prop = "a"; // equivalent to:
if (s is not Nullable) // pseudo code
{
    // Do nullable reference type check
}
else if (s.HasValue)
{
    s = s.Value with { Prop = "a" };
}

with expression

Of course the real question being "should it be included"? I would say either "yes" or "not completely no"... the with expression makes this tricky. With solves the problem of modifying record and (as of c#10) structure types: cool, we can assign stuff in a single block now. Then, what about this?

S? ns;
S s;
s = ns with { Prop = "a" }; // doesn't compile, should be:

if (ns.HasValue)
{
    s = ns.Value with { Prop = "a" };
}

But that sucks! I want the ?. operator to handle it! So then it should be:

s = ns?.Value with { Prop = "a" }; // won't execute if null.

But if we implement that, we might as well add

s?.Prop = "a"; // evaluates to:
s = s?.Value with { Prop: "a" }; // evaluates to:
if (s.HasValue)
{
    s = s.Value with { Prop: "a" };
}

Well great, we're there. Or are we? Just to throw one more wrinkle in the mix, what happens if we do this?

S? ns;
S? ns2 = new S();
ns2 = ns?.Value with { Prop = "a" }; // Should this return null or leave ns2 unmodified?

I don't know the answer, but I do like the discussion. Single line assignment cases like this aren't going away, the community will keep asking until all of them are eradicated. The same thing happened with events, which after many tantrums turned into the now beautiful standard event?.Invoke();

@jcouv jcouv assigned RikkiGibson and unassigned jcouv and CyrusNajmabadi Oct 10, 2022
@BillWagner
Copy link
Member

Would this syntax be legal, and would it force the right hand side to be evaluated:

c?.E ?? _ = ExprWithSideEffects();

@RikkiGibson
Copy link
Contributor Author

RikkiGibson commented Oct 10, 2022

I think it is not legal, because we don't define ?? as a conditional operation (i.e. something which is grouped under the right-side of a ? operator, such as .M() in a?.M() or [i] in a?[i]). So you end up back in the case the language says you are trying to assign to something which is not an lvalue.

I think if you wanted semantics like "always do this, and assign it to some target if present" we would want you to do something like

var result = ExprWithSideEffects();
c?.E = result;

@RikkiGibson
Copy link
Contributor Author

Regarding #6045 (comment)

Thanks for the very interesting discussion. It feels like nullable value types and reference types are maybe experiencing an analogous issue here. SharpLab

C? Method1(C? input)
{
    return input with { field = 43 }; // warning: 'input' may be null
}

S? Method2(S? input)
{
    return input.Value with { field = 43 }; // warning: Nullable value type may be null.
}

record C { public int field; }
struct S { public int field; }

In the original proposal, we are improving the ergonomics of "update this thing if it not null", but we are maybe leaving behind "create an updated version of this thing if it is not null". The closest thing to what we really want is still input == null ? null : input with { ... }.

I'm not certain whether/how to address but I think we should keep an eye on it and see if people keep hitting it.

@jnm2
Copy link
Contributor

jnm2 commented Oct 26, 2022

A discussion of with on nullable things has also started here: #6575

@CaseyHofland
Copy link

CaseyHofland commented Oct 27, 2022

“The closest thing to what we really want is still input == null ? null : input with { ... }.”

Considering that I recognize it as its separate feature now. Thanks @jnm2 for the link, I’ve followed up!
Does null assignment syntax sugar for value types still hold? That just seems like an if statement we would otherwise write ourselves.

@BillWagner
Copy link
Member

Inspired by the ECMA committee discussion on dotnet/csharpstandard#664

As we do this, we need to define what happens in a case like this:

(a?.B, c?.D) = (e.M1(), f.M2());

Whatever we do, we should be consistent with the current evaluation order for tuple / deconstruction assignment:

(x, y) = (y, x); // swap

I'll suggest that the order of evaluation (non-spec language) be:

  1. Evaluate the left side expressions in left-to-right order to determine which are variables and which are null.
  2. For all left side expression variables, evaluate the right side expressions in left-to-right order. Note that if all left expressions are null, none of the right side expressions are evaluated.
  3. Perform the necessary assignment in left-to right order.

/cc @MadsTorgersen

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Nov 3, 2022

afaict, this is not null-conditional-assignment. It doesn't match teh grammar for that feature. Specifically, the grammar is:

null_conditional_assignment
    : null_conditional_member_access assignment_operator expression
    : null_conditional_element_access assignment_operator expression

Neither of which match the above. As such, this is normal assignment. As such, the LHS doesn't produce LValues to assign into, so it's simply illegal and doesn't compile.

@HaloFour
Copy link
Contributor

@SF-Simon

If you have specific syntax proposals that you would like to suggest based on Swift feel free to open new discussions.

Swift does treat optionals differently than C#, though, in the sense that you are more or less forced to unwrap them into a different identifier. You don't have to do that in C#, and the idiomatic null check x != null or x is not null is generally more concise.

@HaloFour
Copy link
Contributor

@SF-Simon

I hope I can persuade everyone with a little words and change this situation.

I'm suggesting that you represent the change that you would like to see, open discussions around features or syntax that you would like to see C# implement. Guidance like "be like Swift" isn't really actionable, but specific examples of syntax that you think would make development simpler could be considered.

@tilkinsc
Copy link

tilkinsc commented Jan 19, 2024

The proposal should clarify this circumstance,

if ((objName?.a = func()) == null)
{
	
}

In which null-aware assignments in an if statement condition will return:

  • assignment lvalue is null (any variable?. is null), null
  • assignment rvalue is null, null
  • assignment lvalue/rvalue is not null, func()

Assignments still must follow the code of:

  • if lvalue is null, rvalue is evaluated, and assignment happens
  • if lvalue is not null, rvalue is not evaluated, and no assignment happens

Its important to keep the idea that null should be emplaced where (objName?.a = func()) is:

if (null == null)
{

}

Actually, it was pointed out that this is defined in the proposal,
https://github.com/dotnet/csharplang/blob/main/proposals/null-conditional-assignment.md#specification

However, I think a welcome change is to add code too in addition to grammar.

@Rekkonnect
Copy link
Contributor

To me this looks implied from the semantics that the expression has when it's not an expression statement; meaning the expression's value is used. The same would apply to all contexts, including boolean variable assignments, if statements, while statements, do-while statements, for statements, etc.

The semantics are pretty clear for this case, even though it's probably best to never write code like that and split out the assignment's result into a variable and then compare it to null.

@jamesford42
Copy link

This is gold, can't tell you guys how many times I've written assign foo to bar if bar isn't null with ?. and had to remember it's not possible.

@Rekkonnect
Copy link
Contributor

Are there any chances this is being included in C# 13? We've been waiting for this improvement for quite a long time and it's a massive QoL. Last time this was touched by the LDM was in 2022.

@333fred
Copy link
Member

333fred commented Jun 19, 2024

Very unlikely at this point.

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

No branches or pull requests