100

The following code results in use of unassigned local variable "numberOfGroups":

int numberOfGroups;
if(options.NumberOfGroups == null || !int.TryParse(options.NumberOfGroups, out numberOfGroups))
{
    numberOfGroups = 10;
}

However, this code works fine (though, ReSharper says the = 10 is redundant):

int numberOfGroups = 10;
if(options.NumberOfGroups == null || !int.TryParse(options.NumberOfGroups, out numberOfGroups))
{
    numberOfGroups = 10;
}

Am I missing something, or is the compiler not liking my ||?

I've narrowed this down to dynamic causing the issues (options was a dynamic variable in my above code). The question still remains, why can't I do this?

This code doesn't compile:

internal class Program
{
    #region Static Methods

    private static void Main(string[] args)
    {
        dynamic myString = args[0];

        int myInt;
        if(myString == null || !int.TryParse(myString, out myInt))
        {
            myInt = 10;
        }

        Console.WriteLine(myInt);
    }

    #endregion
}

However, this code does:

internal class Program
{
    #region Static Methods

    private static void Main(string[] args)
    {
        var myString = args[0]; // var would be string

        int myInt;
        if(myString == null || !int.TryParse(myString, out myInt))
        {
            myInt = 10;
        }

        Console.WriteLine(myInt);
    }

    #endregion
}

I didn't realize dynamic would be a factor in this.

Juho Rutila
  • 2,316
  • 1
  • 25
  • 40
Brandon Martinez
  • 1,330
  • 1
  • 9
  • 13
  • Don't think it's smart enough to know that you aren't using the value passed in to your `out` param as input – Charleh Apr 30 '13 at 17:05
  • Where is the error? Is it in the use of `numberOfGroups` *after* the code you've shown? Can you change the question to show a short but complete program demonstrating the problem? – Jon Skeet Apr 30 '13 at 17:08
  • 3
    The code given here does not demonstrate the described behaviour; it works just fine. Please post code that *actually demonstrates the behaviour you're describing* that we can compile ourselves. Give us the whole file. – Eric Lippert Apr 30 '13 at 17:22
  • 8
    Ah, now we have something interesting! – Eric Lippert Apr 30 '13 at 17:24
  • @EricLippert: The dynamic factor is baffling! :) – leppie Apr 30 '13 at 17:24
  • 1
    It's not too surprising that the compiler is confused by this. The helper code for the dynamic call site probably has some control flow that don't guarantee assignment to the `out` param. It's certainly interesting to consider what helper code the compiler should produce to avoid the issue, or if that's even possible. – CodesInChaos Apr 30 '13 at 17:26
  • 1
    At first glance this is sure looking like a bug. – Eric Lippert Apr 30 '13 at 17:30
  • Interestingly, it seems to be the `myString == null` that's the sticking point. The following condition *also* results in the error: `if (myString == null || (myInt = 3) == 3) { ... }`. As does `if (myString == null || myString != null)`. – dlev Apr 30 '13 at 17:30
  • @CodesInChaos: Deleted my previous comment, obviously I cant look at the IL! DOH ;p – leppie Apr 30 '13 at 17:33
  • @CodesInChaos removing the `myString == null` and relying just on `TryParse` (which is what I since changed my code to), the code compiles fine. It's my understanding that `out` has to be guaranteed (and thus, a possible bug somewhere in there). – Brandon Martinez Apr 30 '13 at 17:35
  • @leppie: set myInt to 0 in the initializer and then look at the IL. – Eric Lippert Apr 30 '13 at 17:56
  • @EricLippert: Why would you be so cruel?!! Dammit, that is some ugly code ;p I am glad IronScheme's code, although mostly dynamic typed, is much easier to read via your favourite decompiler :) – leppie Apr 30 '13 at 18:04

3 Answers3

73

I am pretty sure this is a compiler bug. Nice find!

Edit: it is not a bug, as Quartermeister demonstrates; dynamic might implement a weird true operator which might cause y to never be initialized.

Here's a minimal repro:

class Program
{
    static bool M(out int x) 
    { 
        x = 123; 
        return true; 
    }
    static int N(dynamic d)
    {
        int y;
        if(d || M(out y))
            y = 10;
        return y; 
    }
}

I see no reason why that should be illegal; if you replace dynamic with bool it compiles just fine.

I'm actually meeting with the C# team tomorrow; I'll mention it to them. Apologies for the error!

Pierre Arnaud
  • 10,212
  • 11
  • 77
  • 108
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 6
    I'm just glad to know that I'm not going crazy :) I've since updated my code to only rely on TryParse, so I'm set for now. Thanks for your insight! – Brandon Martinez Apr 30 '13 at 17:39
  • @EricLippert, is this a bug or rather a design flaw? Since as my answer points out, the compiler is designed *not* to resolve any expression with a `dynamic` variable. Perhaps the `bool` case needed to be treated separately? – NominSim Apr 30 '13 at 17:43
  • @BrandonMartinez: Or, just initialize the local. – Eric Lippert Apr 30 '13 at 17:50
  • 4
    @NominSim: Suppose the runtime analysis fails: then an exception is thrown before the local is read. Suppose the runtime analysis succeeds: then *at runtime* either d is true and y is set, or d is false and M sets y. Either way, y is set. The fact that the analysis is deferred until runtime doesn't change anything. – Eric Lippert Apr 30 '13 at 17:52
  • @EricLippert I think you misunderstand me, I was agreeing that it is a flaw, but the compiler is designed per the language spec to ignore expressions that contain dynamic variables. It seems rather that it should have a special case analysis for bool expressions, or perhaps better an error message that explains that the deferred analysis of the expression could result in an unassigned variable. – NominSim Apr 30 '13 at 18:09
  • 2
    In case anyone's curious: I just checked, and the Mono compiler gets it right. http://imgur.com/g47oquT – Dan Tao Apr 30 '13 at 18:34
  • 17
    I think the compiler behavior is actually correct, since the value of `d` may be of a type with an overloaded `true` operator. I've posted an answer with an example where neither branch is taken. – Quartermeister Apr 30 '13 at 19:14
  • I cannot reproduce with OP's example, but this one, within `dynamic`. – Ken Kin May 06 '13 at 06:51
  • 2
    @Quartermeister in which case the Mono compiler is getting it wrong :) – porges May 07 '13 at 20:56
52

It's possible for the variable to be unassigned if the value of the dynamic expression is of a type with an overloaded true operator.

The || operator will invoke the true operator to decide whether to evaluate the right-hand side, and then the if statement will invoke the true operator to decide whether to evaluate the its body. For a normal bool, these will always return the same result and so exactly one will be evaluated, but for a user-defined operator there is no such guarantee!

Building off of Eric Lippert's repro, here is a short and complete program that demonstrates a case where neither path would be executed and the variable would have its initial value:

using System;

class Program
{
    static bool M(out int x)
    {
        x = 123;
        return true;
    }

    static int N(dynamic d)
    {
        int y = 3;
        if (d || M(out y))
            y = 10;
        return y;
    }

    static void Main(string[] args)
    {
        var result = N(new EvilBool());
        // Prints 3!
        Console.WriteLine(result);
    }
}

class EvilBool
{
    private bool value;

    public static bool operator true(EvilBool b)
    {
        // Return true the first time this is called
        // and false the second time
        b.value = !b.value;
        return b.value;
    }

    public static bool operator false(EvilBool b)
    {
        throw new NotImplementedException();
    }
}
Quartermeister
  • 57,579
  • 7
  • 124
  • 111
  • This is pretty neat, but I'm not sure it applies to the OP's case, since there is an equality operator being invoked; I don't think there's a chance to call a custom `operator true` in that case. – dlev Apr 30 '13 at 19:43
  • @dlev: The equality expression is also dynamically bound, and could bind to a user-defined operator that returns something other than `bool`. – Quartermeister Apr 30 '13 at 19:47
  • Oops, good call; forgot `operator==` doesn't need to return a `bool`. – dlev Apr 30 '13 at 19:52
  • 8
    Nice work here. I've passed this along to the C# test and design teams; I'll see if they have any comments on it when I see them tomorrow. – Eric Lippert Apr 30 '13 at 20:47
  • 3
    This is very strange to me. Why should `d` be evaluated twice? (I'm not disputing that it clearly *is*, as you've shown.) I would have expected the evaluated result of `true` (from the first operator invocation, cause by `||`) to be "passed along" to the `if` statement. That's certainly what would happen if you put a function call in there, for example. – Dan Tao Apr 30 '13 at 21:23
  • 3
    @DanTao: The expression `d` is evaluated only once, as you expect. It's the `true` operator that's being invoked twice, once by `||` and once by `if`. – Quartermeister Apr 30 '13 at 21:59
  • 2
    @DanTao: It might be more clear if we put them on separate statements as `var cond = d || M(out y); if (cond) { ... }`. First we evaluate `d` to get an `EvilBool` object reference. To evaluate the `||`, we first invoke `EvilBool.true` with that reference. That returns true, so we short-circuit and don't invoke `M`, and then assign the reference to `cond`. Then, we move on to the `if` statement. The `if` statement evaluates its condition by calling `EvilBool.true`. – Quartermeister Apr 30 '13 at 21:59
  • 1
    I guess my point is that I would expect `cond` in your example to be *an actual `bool`*, and **not** an `EvilBool`. In other words I was under the (apparently wrong) impression that an expression containing the `||` operator always evaluated to either `true` or `false`, and that overloading the `true` operator only gave you control over which of those two options it would be. – Dan Tao Apr 30 '13 at 22:11
  • Now here's something very curious: if I compile `dynamic d = new EvilBool(); Console.WriteLine(d || true);` I see `EvilBool` in the console. **However**, if I change `dynamic` to `var`, I get a compile-time error, saying "Operator '||' cannot be applied to operands of type 'EvilBool' and 'bool'." Do you see the same thing? (I'm using Mono.) If so, that seems to reinforce to me that something is amiss here. – Dan Tao Apr 30 '13 at 22:14
  • @DanTao: Yeah, I cheated a little bit :). It looks like `dynamic` doesn't check for the `|` operator if it short-circuits. You will get the error in the dynamic version if you have `operator true` return false instead of true. To make `||` actually work, you would need to define `public static EvilBool operator |(EvilBool l, EvilBool r)`, and to make it work with a `bool` you would then also need `public static implicit operator EvilBool(bool b)`. (I left them out in the example because they weren't strictly necessary, and I wanted EvilBool to live up to its name.) – Quartermeister Apr 30 '13 at 22:45
  • 2
    Now this is really cool. I had no idea there is true or false operator. – IS4 May 04 '13 at 21:49
  • @Quartermeister: I'd include the operator. It clarifies that this is not an issue with `dynamic` per se. Here's a gist that includes the appropriate modifications: https://gist.github.com/Porges/5536010 – porges May 07 '13 at 20:51
7

From MSDN (emphasis mine):

The dynamic type enables the operations in which it occurs to bypass compile-time type checking. Instead, these operations are resolved at run time. The dynamic type simplifies access to COM APIs such as the Office Automation APIs, and also to dynamic APIs such as IronPython libraries, and to the HTML Document Object Model (DOM).

Type dynamic behaves like type object in most circumstances. However, operations that contain expressions of type dynamic are not resolved or type checked by the compiler.

Since the compiler does not type check or resolve any operations that contain expressions of type dynamic, it cannot assure that the variable will be assigned through the use of TryParse().

Community
  • 1
  • 1
NominSim
  • 8,447
  • 3
  • 28
  • 38
  • If the first condition is met, `numberGroups` is assigned (in the `if true` block), if not, the second condition guarantees assignment (via `out`). – leppie Apr 30 '13 at 17:36
  • 1
    That is an interesting thought, *but* the code compiles fine without the `myString == null` (relying only the `TryParse`). – Brandon Martinez Apr 30 '13 at 17:36
  • 1
    @leppie The point is that since the first condition (indeed therefore the entire `if` expression) involves a `dynamic` variable, it is not resolved at compile time (the compiler therefore can't make those assumtions). – NominSim Apr 30 '13 at 17:39
  • @NominSim: I see your point :) +1 Could be a sacrifice from the compiler (breaking C# rules), but other suggestions seems to imply a bug. Eric's snippet shows this is a not a sacrifice, but a bug. – leppie Apr 30 '13 at 17:44
  • @NominSim This can't be right; just because certain compiler functions are deferred doesn't mean all of them are. There's plenty of evidence to show that under slightly different circumstances, the compiler does the definite assignment analysis without issue, despite the presence of a dynamic expression. – dlev Apr 30 '13 at 18:02
  • @dlev Can you provide an example of an operation that contains a dynamic expression that is resolved by the compiler? It's my understanding that they are all bypassed by the compiler. – NominSim Apr 30 '13 at 18:16
  • Consider that in this case, if you omit the `myString == null`, then the `out` parameter on the `TryParse()` call *is* enough to satisfy the definite assignment rules. So clearly even though exactly which overload is called *isn't* resolved, the compiler is smart enough to reason that either a) it won't be able to resolve an overload, so an exception will be thrown, or b) whatever overload is found, it will have an `out` parameter that assigns the local. – dlev Apr 30 '13 at 18:42
  • @dlev That's the whole point of my answer...the fact that there is a dynamic expression makes the compiler ignore the entire expression in which it is contained. As EricLippert pointed out this isn't desired behavior, but it *is* the reason why the OP sees an "unassigned local variable" error. – NominSim Apr 30 '13 at 18:45
  • But I'm saying the opposite: it is very clearly *not* ignoring the `TryParse()` expression, since without the `myString == null` part, the local is considered definitely assigned by the very much dynamic expression `int.TryParse(dynamicVariableHere, out localVariable)`. – dlev Apr 30 '13 at 18:51
  • @dlev Oh I see what you are talking about now...The dynamic variable is not part of the if expression when you remove the `myString == null` part however, which is why `TryParse()` is not ignored. – NominSim Apr 30 '13 at 19:15