11

Let's assume I have a class that has a property of type Dictionary<string,string>, that may be null.

This compiles but the call to TryGetValue() could throw at a NullRef exception at runtime:

MyClass c = ...;
string val;
if(c.PossiblyNullDictionary.TryGetValue("someKey", out val)) {
    Console.WriteLine(val);
}

So I'm adding a null-propagating operator to guard against nulls, but this doesn't compile:

MyClass c = ...;
string val;
if( c.PossiblyNullDictionary ?. TryGetValue("someKey", out val) ?? false ) {

    Console.WriteLine(val); // use of unassigned local variable

}

Is there an actual use case where val will be uninitialized inside the if block, or can the compiler simply not infer this (and why) ?

Update: The cleanest (?) way to workaround^H^H^H^H^H fix this is:

MyClass c = ...;
string val = null; //POW! initialized.
if( c.PossiblyNullDictionary ?. TryGetValue("someKey", out val) ?? false ) {

    Console.WriteLine(val); // no more compiler error

}
Erik Philips
  • 48,663
  • 7
  • 112
  • 142
Cristian Diaconescu
  • 31,686
  • 28
  • 127
  • 219
  • What version of C# are you using? If you're using C# 7.0, have you tried using `out string val`, doing away with the declaration before the `if`? – Geoff James Dec 06 '17 at 22:56
  • Interesting. This also occurs when using C# 7's inline declarations for out parameters. I'm assuming it's because the compiler doesn't attempt to determine the boolean value of the if statement at compile-time. ie, how would it know if the variable would have been declared and initialized if you had a logical OR after the first expression. Something like `if(c.PossiblyNullDictionary?.TryGetValue("someKey", out val) ?? false || new Random().Next(0,100) % 2 == 0)` – Jonathon Chase Dec 06 '17 at 22:57
  • @JonathonChase - seems like we commented at virtually the same time - looks like you answered my question! :) – Geoff James Dec 06 '17 at 22:57
  • @GeoffJames I hit this before on C# 6 projects. This time it's just a throwaway "script" (long live Linqpad) so C#7 is ok - but I just tried your suggestion and I get the same compiler error. – Cristian Diaconescu Dec 06 '17 at 23:00
  • 3
    The complier prolly doesn't care/know about the semantics of the result of **TryGetValue**. It only cares that calling it will initialise *val*. If TryGetValue is not called at all you can't assume it was initialised properly. – user6144226 Dec 06 '17 at 23:03
  • @GeoffJames Actually, having an inline out param is worse: for a normal local variable, I can manually initialize it to 'null' or something at declaration time, which I (don't think I) can do with an inline var. – Cristian Diaconescu Dec 06 '17 at 23:03
  • 1
    @user6144226 Doesn't matter what TryGetValue returns as long as it runs. The C# spec forces any method that takes an `out` parameter to initialize it before returning - otherwise the method doesn't compile. – Cristian Diaconescu Dec 06 '17 at 23:05
  • 3
    @CristiDiaconescu that's exactly my point if you use *?.* TryGetValue doesn't get called. – user6144226 Dec 06 '17 at 23:06
  • @CristiDiaconescu - you can't initialize an inline `out var`, as it's only declared and initialized at that point in the code, for the scope of the statement that follows. I wouldn't say it was worse - I would argue it's a use-case scenario, depending on what requirements you have. If you're only going to use it within the statement, it's fine; if you're needing it again outside of the statement, then declaring it before is absolutely necessary. – Geoff James Dec 06 '17 at 23:11
  • @GeoffJames look at my update pls, it explains my remark. I have nothing against inline out vars in general - I was complaining about this particular case. – Cristian Diaconescu Dec 06 '17 at 23:15
  • @CristiDiaconescu - totally agree, I was just mentioning it as it might be useful for other users :) – Geoff James Dec 06 '17 at 23:15
  • @itsme86, for this case sure. I'll concede that this makes a lot of sense. But for this to work an expression has to be supplied to *??*. Any expression can be supplied after *??* including something undeterministic like *Rand() % 2 ==0*. What should the complier do? – user6144226 Dec 06 '17 at 23:16
  • I think folks are missing the reason the compiler's complaining, which it would do regardless of whether a `.?` or `??` is involved; I'd be curious though if the same behavior was observed with a "real" reference type instead of string – Josh E Dec 06 '17 at 23:17
  • @JoshE Why would a string not be a 'real' ref-type for the intents and purposes of this example? – Cristian Diaconescu Dec 06 '17 at 23:19
  • mmm I was thinking of string's value-type semantics, I guess if it behaved differently it would be an indication of a bug in the compiler -- an unlikely premise – Josh E Dec 06 '17 at 23:20
  • 3
    @user6144226 Sure, but the compiler is traditionally pretty good at figuring out constant-valued expressions. For instance: `if( true || c.Dict.TryGetValue(key, out val) ) // compiler error, uninitialized var` but `if( false || c.Dict.TryGetValue(key, out val) ) // works` – Cristian Diaconescu Dec 06 '17 at 23:23
  • isn't that just the compiler noting that `false || == false` and optimizing away except the const false? – Josh E Dec 06 '17 at 23:30
  • @CristiDiaconescu: That is because in that case it is a compile time constant known expression. If you extract the `false` into a variable then the compiler stops being able to figure it out. This is pretty good evidence that the compiler never tries to work out what a variable will contain at any point in the program. – Chris Dec 06 '17 at 23:33
  • 1
    @Chris The point is, the OP had the compile time constant `false` in the expression and it *still* can't figure it out. It's not `Rand() %2 == 0` or anything nutty. It's the compile time constant. – itsme86 Dec 06 '17 at 23:37
  • @JoshE `false || ` is ``, not `false`. Now, `true || `... – Cristian Diaconescu Dec 06 '17 at 23:41
  • @itsme86: I'm not sure what your point is. In the case with the `false` in the compiler *knows* that it will be running the `TryGetValue` at compile time so use of `val` is allowed. If you replace the `false` with a local variable set to false the compiler stops being able to make that conclusion and you get the unassigned variable compiler error. – Chris Dec 06 '17 at 23:41
  • @Chris The point is that a variable (noun) is, well, variable (adj). What I have is a compile-time constant. Try this: `const bool ct = false; string val; if(ct || dict.TryGetValue("", out val)) { cw(val); }` - no compile error. Admittedly, this is a different use case than what I raised in the question so not apples-to-apples. But it feels (to me) like the compiler *could* figure out the transitive closure of this call graph. – Cristian Diaconescu Dec 06 '17 at 23:45
  • @CristiDiaconescu: Can you explain why you would expect a compile time error? That is no difference from your earlier example of `if( false || c.Dict.TryGetValue(key, out val))` which the compiler will interpret as `if(c.Dict.TryGetValue(key, out val))` and thus it knows `val` will be initialised. The reason this works is because the compiler can simplify the expression because ct is false at compile time. a non-const variable though is by definition as you say variable and thus the compiler refuses to make any assumptions about its runtime value. – Chris Dec 06 '17 at 23:49
  • 1
    @CristiDiaconescu - re: true || - d'oh of course. My bad – Josh E Dec 06 '17 at 23:52

3 Answers3

5

Seems like you have run into a limitation of the compilers understanding of ?. and ?? which isn't too surprising, given that they aren't really fully incorporated in the language.

If you make your test explicit without the newer operators, the compiler will agree with you:

MyClass c = new MyClass();
string val;
if (c.PossiblyNullDictionary != null && c.PossiblyNullDictionary.TryGetValue("someKey", out val)) {
    Console.WriteLine(val); // now okay
}
NetMage
  • 22,242
  • 2
  • 28
  • 45
  • You can suggest a compiler improvement, assuming it isn't too complicated it may get done someday. They are talking about adding `Expression` support for `?.` and `??` so this could also be added in... – NetMage Dec 06 '17 at 23:13
  • 2
    I think the OP is aware of this approach, but is trying to avoid having to have a larger-than-necessary `if` condition – Geoff James Dec 06 '17 at 23:13
  • @NetMage can you explain what you mean about the features being "[not] fully incorporated into the language"? Aren't they part of the C# specification? – Josh E Dec 07 '17 at 18:22
  • One (commonly mentioned) example is they are not supported by `Expression`. Obviously another example is this one, the compiler doesn't understand `?.` / `??` the way it does `&&` and `||`. The LINQ to SQL/Entities can't convert `??` even though SQL has the `COALESCE` operator. Etc. – NetMage Dec 07 '17 at 20:53
  • those seem like good examples, but they seem to me like they're better examples of .NET libraries not offering support for new C# language features, which has nothing to do with the compiler. – Josh E Dec 07 '17 at 22:29
  • True, except for this one. Also, the compiler must understand `??` since it converts a lambda to an expression tree, so a bit of both I think. Also, I think a full implementation would include `??=` which is definitely compiler/language. Adding a binary operator and not adding the combined assignment operator strikes me as not fully implemented. – NetMage Dec 07 '17 at 23:07
  • I guess I'm missing your meaning a bit - `??` is already an assignment operator. Also, if the features you mention aren't fully implemented, that would mean that the standard ternary operator is not as well -- you will get the exact same compiler error if you replace the original statement with `c.PossiblyNullDictionary == null ? false : c.PossiblyNullDictionary.TryGetValue("someKey", out val)`. IOW, it's not the language elements newness, it's the fact that the statements are separated in your answer that allows the compiler to gurantee the init of `val` – Josh E Dec 08 '17 at 17:38
  • 1
    No, `??` is not an assignment operator. `??=` is an assignment operator, but it isn't yet implemented. The fact that the `&&` test works, which I wouldn't call separated, seems to indicate that the ternary operator isn't processed completely either. – NetMage Dec 08 '17 at 18:05
3

By initializing val to a erhm, value (e.g., String.Empty) the compiler is able to grok the intent for the null operators and behaves as expected (via LINQPad, natch):

void Main()
{
    MyClass c = new MyClass();
    string val = string.Empty;
    if (c.PossiblyNullDictionary?.TryGetValue("someKey", out val) ?? false)
    {

        Console.WriteLine(val);

    }
}
public class MyClass {
    public Dictionary<string, string> PossiblyNullDictionary;
}
// Define other methods and classes here

Ed: by 'grok the intent' I meant that the compiler can't make important guarantees about the program's characteristics if it allows execution to leave the current scope with val uninitialized. When it evaluates the null operators, the method invocation.

The use case you ask for is this: Say that instead of TryGetValue, we have bool SomeMethod(string s, out v). Let's say that when invoked, SomeMethod is naughty and simply has a body of return true;. The compiler treats method invocation bodies as opaque (since it may not always be in an assembly available/visible to the compiler), so it concludes that there's no way to prove that val is ever initialized.

ed: In response to some comments, I wanted to update my answer to point out that this behavior isn't specific to the ?? or ?. C# language features; you can reproduce the same effect simply by using a ternary expression:

c.PossiblyNullDictionary == null ? 
    false : 
    c.PossiblyNullDictionary.TryGetValue("someKey", out val) 
 //error: use of possibly uninitialized local variable
Josh E
  • 7,209
  • 2
  • 30
  • 43
  • Yeah, that's sort of what I usually end up doing, see my update at the end of the Q. I was just wondering if I'm missing something and the compiler really is right (it usually is about this kind of errors!) – Cristian Diaconescu Dec 06 '17 at 23:16
  • 1
    @CristiDiaconescu: I think what the compiler is right about is that it doesn't know that it is initialized. if you do `bool val = true; string test; if (val){test = "hoorah!";} Console.WriteLine(test);` then the compiler tells you that `test` is not initialized. I think the problem is that the compiler considers that the result of the if could be either true or false. Just because *we* know that it is going to either be false or it will have run TryGetValue doesn't mean the compiler looks that far. I assume this is because this is a rabbithole that the compiler just refuses to start down. – Chris Dec 06 '17 at 23:21
  • what @chris said is what I was starting to write myself, albeit less coherently – Josh E Dec 06 '17 at 23:22
  • What I'll add is that the compiler doesn't know what `TryGetValue` does, just that it returns a boolean and that it is passed a kind-of ref. it doesn't know how the function will mutate the `val` reference – Josh E Dec 06 '17 at 23:28
  • 4
    @JoshE: Not true. The compiler knows that all out variables will be initialised when the function call returns. This is a requirements of out parameters (its a compiler error not to assign to it before you return from a method). – Chris Dec 06 '17 at 23:30
  • indeed, the compiler enforces that requirement because otherwise it lose the ability to make that guarantee (I admit that sounds suspiciously like a tautology) – Josh E Dec 06 '17 at 23:31
  • Which makes your example of `SomeMethod` invalid - there is no way to `return true;` as the body of `SomeMethod` - it must initialize `v` when called. – NetMage Jun 30 '20 at 00:41
-3

That's because if c.PossiblyNullDictionary is null, TryGetValue won't be executed and that expression won't return true or false.

c.PossiblyNullDictionary ?. TryGetValue("someKey", out val) returns Nullable, you can replace your code with something like this and will compile:

        string val;
        var result = c.PossiblyNullDictionary?.TryGetValue("key", out val);
        if (result.HasValue && result.Value)
        {

        }
olegk
  • 695
  • 4
  • 13
  • 1
    ...hence the `?? false` at the end. – Cristian Diaconescu Dec 06 '17 at 23:04
  • You seem like you are half right except that the problem is that because `TryGetValue` isn't executed then `val` will never be initialised. – Chris Dec 06 '17 at 23:04
  • The `?.` null propagation will take care of the `PossiblyNullDictionary` being null and not executing `TryGetValue` - resulting in a `null` evaluation instead. The null coalescent operator `??` will handle that null, reverting to `false` in this case. – Geoff James Dec 06 '17 at 23:05
  • 1
    Look at Geoff's comment to your question, he beat me to it. – Cristian Diaconescu Dec 06 '17 at 23:08
  • @olegk and your updated answer still has the same error... use of unassigned local variable `val` – NetMage Dec 06 '17 at 23:09
  • Your updated code is a longer version of my one-liner. Note the `?? false` null-coalescing operator at the end. If the dictionary is `null` the evaluation goes like this: `if(c.null ?.TryGV() ?? false){...}` === `if(null ?? false){...}` === `if(false){...}` – Cristian Diaconescu Dec 06 '17 at 23:10