C# 14: Null conditional assignments.

[michaelnebel]

In this PR, we:

• Add support for C# 14 null conditional assignments. While the extractor already supported this feature, updates to the C# control flow implementation were needed.
• Simplify the control flow logic for qualified write access, which also helps with the addition of null conditional assignments.
• Make a small improvement to the MaybeNullExpr class so that it now correctly handles null conditional (read) accesses.

Changes to existing control flow

Previously, the control flow for

ca.IntField = 42

looked like this:

ca -> 42 -> ca.IntField = 42

With this PR, we now include the field access after evaluating the right-hand side, so the control flow becomes:

ca -> 42 -> ca.IntField -> ca.IntField = 42

This matches how property assignments are handled, where it’s important for the property access to be included in the control flow after evaluating the right-hand side, since accessing the property corresponds to invoking the property setter.

Previously, the control flow for

M(out ca.IntField)

looked like this:

ca -> call M

With this PR, we now include the field access after the call, so the control flow becomes:

ca -> call M -> ca.Field

New control flow.

The changes to the control flow graph implementation mean that for a statement like

ca?.Prop?.StringProp = "World";

the control flow is now built correctly to reflect the sequence of null conditional accesses.

That is, if the qualifier uses a null conditional, we add a null edge from the qualifier to the point after the assignment. This reflects the fact that if the qualifier is null, the right-hand side of the assignment isn’t evaluated.

[michaelnebel]

@hvitved : Would it be possible to get some early feedback on the contents of the PR (in case we should design it differently)?

[hvitved]

Overall LGTM, I mostly have a concern about CFGs for out arguments.

[hvitved]

This QL doc is now incorrect. Perhaps just remove the entire In a qualified write access, the access itself is not evaluated, only the qualifier and the indexer arguments (if any).

[hvitved]

if i = 0 then not c.(NullnessCompletion).isNull() else any() is slightly better as it avoids tuple duplication during evaluation (the case where i != 0 and c is not (NullnessCompletion).isNull() is currently valid for both disjuncts.

[hvitved]

Accessor means stuff like get and set, so that is no longer the right word to use as it also includes fields.

[hvitved]

Same comment.

[hvitved]

Is this actually needed? I would think that an expression like x?.M() can always potentially be null, regardless of what we know about x.

[hvitved]

call M -> M(out x.Field)

[hvitved]

Actually, I'm not convinced that this is the CFG we want in this case. Consider this example:

    void MyTest()
    {
        if (Set(out this.IntField))
            return;
        Guarded();
    }

It gets the following CFG

flowchart TD
1["enter MyTest"]
10["return ...;"]
11["call to method Guarded"]
12["this access"]
13["...;"]
2["exit MyTest"]
3["exit MyTest (normal)"]
4["{...}"]
5["if (...) ..."]
6["call to method Set"]
7["this access"]
8["this access"]
9["access to field IntField"]

1 --> 4
3 --> 2
4 --> 5
5 --> 7
6 -- false, true --> 9
7 --> 8
8 --> 6
9 -- false --> 13
9 -- true --> 10
10 -- return --> 3
11 --> 3
12 --> 11
13 --> 12

Loading

Note how get the join at access to field IntField, which means that we can no longer tell that Guarded is guarded by Set(out this.IntField) evaluating to true. I think what we want is simply the same as if there was no out, i.e.

flowchart TD
1["enter MyTest"]
10["return ...;"]
11["call to method Guarded"]
12["this access"]
13["...;"]
2["exit MyTest"]
3["exit MyTest (normal)"]
4["{...}"]
5["if (...) ..."]
6["call to method Set"]
7["this access"]
8["this access"]
9["access to field IntField"]

1 --> 4
3 --> 2
4 --> 5
5 --> 7
9 --> 6
7 --> 8
8 --> 9
6 -- false --> 13
6 -- true --> 10
10 -- return --> 3
11 --> 3
12 --> 11
13 --> 12

Loading

[michaelnebel]

Yes, we can do it like that. However, then it looks like a field read instead.

[michaelnebel]

It will also significantly simplify the implementation.