Null-coalescing out parameter gives unexpected warning
Asked Answered
A

3

11

Using this construct:

var dict = new Dictionary<int, string>();
var result = (dict?.TryGetValue(1, out var value) ?? false) ? value : "Default";

I get an error saying CS0165 use of unassigned local variable 'value' which is not what I expect. How could value possibly be undefined? If the dictionary is null the inner statement will return false which will make the outer statement evaluate to false, returning Default.

What am I missing here? Is it just the compiler being unable to evaluate the statement fully? Or Have I messed it up somehow?

Aras answered 17/3, 2019 at 12:25 Comment(2)
Special states "Definitely assigned after true expression" or "Definitely assigned after false expression" are only tracked for limited numbers of operators. As far as I understand, ?. and ?? are not among them. You could use (dict != null && dict.TryGetValue(1, out var value)) ? value : "Default" instead.Renewal
Yes, that is what I did, I also created an extension method to simplify things. Would be nice to be able to use such constructs in for example Linq Query syntax without additional extension methods though.Munda
H
12

Your analysis is correct. It is not the analysis the compiler makes, because the compiler makes the analysis that is required by the C# specification. That analysis is as follows:

  • If the condition of a condition?consequence:alternative expression is a compile-time constant true then the alternative branch is not reachable; if false, then the consequence branch is not reachable; otherwise, both branches are reachable.

  • The condition in this case is not a constant, therefore the consequence and alternative are both reachable.

  • local variable value is only definitely assigned if dict is not null, and therefore value is not definitely assigned when the consequence is reached.

  • But the consequence requires that value be definitely assigned

  • So that's an error.

The compiler is not as smart as you, but it is an accurate implementation of the C# specification. (Note that I have not sketched out here the additional special rules for this situation, which include predicates like "definitely assigned after a true expression" and so on. See the C# spec for details.)

Incidentally, the C# 2.0 compiler was too smart. For example, if you had a condition like 0 * x == 0 for some int local x it would deduce "that condition is always true no matter what the value of x is" and mark the alternative branch as unreachable. That analysis was correct in the sense that it matched the real world, but it was incorrect in the sense that the C# specification clearly says that the deduction is only to be made for compile-time constants, and equally clearly says that expressions involving variables are not constant.

Remember, the purpose of this thing is to find bugs, and what is more likely? Someone wrote 0 * x == 0 ? foo : bar intending that it have the meaning "always foo", or that they've written a bug by accident? I fixed the bug in the compiler and since then it has strictly matched the specification.

In your case there is no bug, but the code is too complicated for the compiler to analyze, so it is probably also too complicated to expect humans to analyze. See if you can simplify it. What I might do is:

public static V GetValueOrDefault<K, V>(
  this Dictionary<K, V> d,
  K key,
  V defaultValue)
{
    if (d != null && d.TryGetValue(key, out var value))
      return value;
    return defaultValue;
}
…
var result = dict.GetValueOrDefault(1, "Default");

The goal should be to make the call site readable; I think my call site is considerably more readable than yours.

Hubbard answered 17/3, 2019 at 15:6 Comment(1)
Agree, your solution is much more readable. I am not sure I want to hide the null check at the call site but it does not hurt checking for null in the extension method anyway. A question though, did you mean to say "The condition in this case is not a constant" in the second bullet?Munda
E
5

Is it just the compiler being unable to evaluate the statement fully?

Yes, more or less.

The compiler does not track unassigned, it tracks the opposite 'defintely assigned'. It has to stop somewhere, in this case it would need to incorporate knowledge about the library method TryGetValue(). It doesn't.

Elapse answered 17/3, 2019 at 12:36 Comment(2)
But all out parameters must be assigned before returning from a method, except if a method throws an exception, right? So the compiler should know that the out value at least has a default value I would expect. If the code said ?? true that would lead to an unassigned value.Munda
Yes, I think @PetSerAl is closer with the ?. and ?? operators not being considered.Elapse
K
1

This should have been fixed in later compilers with Improved Definite Assignment Analysis, see the conditional access coalesced to a bool constant example:

if (c?.M(out object obj4) ?? false)
{
    obj4.ToString(); // undesired error
}

Currently it does not produce any error - Demo @sharplab.io.

Kandacekandahar answered 20/6, 2023 at 10:20 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.