3

If I try and compile this fragment of code I get a "Use of unassigned local variable" for data. (Obviously foo = true was originally more complicated.)

CuppingAttemptData data;
bool purgeData = true;
bool foo = true;

purgeData = foo &&
            dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);


if (purgeData)
{
    DataSerialization.SaveData(new CowID("Missing", "Missing"), data);
}

The use in question is on the second-to-last line: the variable data at the end of the SaveData call. At first glance it would seem that the compiler just can't see that TryDequeue is guaranteed to have set data, but this code does compile:

CuppingAttemptData data;
bool purgeData = true;

purgeData = dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);


if (purgeData)
{
    DataSerialization.SaveData(new CowID("Missing", "Missing"), data);
}

i.e. without my trivial extra condition the compiler can prove that data is set.

So what is going on?

Of course I can make it compile by setting data to null at the top, but I'm a little worried that the compiler is seeing something that I'm not and that data may not be set to anything sensible when I call SaveData.

I'm using Visual Studio 2015 (update 1) with .Net framework 4.6.01055.

Improve this question
4
  • 3
    In second snipped dataQueue.TryPeek(out data) always executed and assign data. In first snipped dataQueue.TryPeek(out data) will not be executed when foo is false, so it possible (from compiler point of view) that data remains unassigned. Commented Feb 16, 2016 at 2:59
  • I guess it's a property of the definite assignment detection algorithm that the C# compiler uses. Unfortunately, the C# spec for recent versions are not available, but obviously it can only go so far in detecting assignment, and I suppose this is an example of what it will not do. Commented Feb 16, 2016 at 3:00
  • 2
    Another interesting fact is that if the expression on right hand side of the purgeData assignment in the first case is moved into the if clause then there is no compiler error either. Put another way, elimination of the purgedData variable resolves the issue as well. Commented Feb 16, 2016 at 3:24
  • +1 for the interesting factoid @mikez :) i suppose that when INSIDE the if condition, the compiler can confidently assume data will have a value. Commented Feb 16, 2016 at 3:26

3 Answers 3

Reset to default
5

The issue is that && is a short circuit operator. Consider your first code segment

purgeData = foo &&
            dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);

Should foo be false for any reason, the other statements in the condition will not be evaluated. Thus, neither TryPeek nor TryDequeue will run. While in your example foo will never be false, the compiler cannot know that with a non-trivial example. Perhaps not even with a trivial example, though a smarter utility like ReSharper may catch it.

Your second snippet compiles because TryPeek is also guaranteed to return data. It actually has nothing to do with TryDequeue at all.

purgeData = dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Of course! I was too busy thinking that the compiler was being smart with the if statement to remember that the out parameter givesdata some value - even if it could be a garbage value.
1

At first glance it would seem that the compiler just can't see that TryDequeue is guaranteed to have set

Oh, it almost certainly can. This is the sort of expression where the C# compiler will omit needless checks, so if you did something valid after foo && then the compiler would compile it as if that foo && wasn't there. Quite likely as if foo wasn't there at all, in fact (in a release build, anyway).

But that's at the level of the compiler, and the bug in your code is at the level of the language.

Consider, the fact that it's possible to cut out foo isn't a feature of the language, it's an optimisation based on the end result.

As far as the C# rules of the language go, foo is a variable. As a variable, it's value may vary. As it may vary, the branch that depends on it may or may not be taken.

Indeed, it's not completely impossible that it could change, if it was captured into an asynchronously executing piece of code, for example.

Yeah, you could say "but it's not captured into an asynchronously executing piece of code", but you have to look at more than just that bit of code to know that.

Programming languages are for people first, computers second. It should be possible to look at a piece of code and know what it means, and the code here means an unassigned variable is possibly accessed. It's great for a clever compiler to change how a program runs, but not what it means: Indeed, that in itself is a test of a bad optimisation, if it changes the meaning it's not an optimisation, it's a bug.

Consider also if this was allowed. Would it be allowed if foo was assigned the value of comparing calculating the factorial of 23 and seeing if it was greater than 2 to the power of 45? That's always true, so should it be considered that a branch after foo && is always taken? If we change the rule from "there must be no branches that can be followed that leave the variable unassigned" to "there must be no branches that can be followed that leave the variable unassigned, being smart about examining those branches", then what level of "smart" should we have? How are we to express that level of smart so that a programmer can know what it is?

Taken to its logical conclusion, we'd have to allow that a C# program would be valid when compiled with one compiler but invalid when compiled with a less smart compiler. And the C# isn't a single programming language any more, but a family of mostly-but-not-entirely-compatible dialects. That's to be avoided if possible.

Even if all the compilers were that smart, we don't want them to be too much smarter than we are in this regard (though we certainly do in some others). If a person can't know themselves if something is valid, how can the write a program?

In all it makes sense that at compile-time only constant expressions can be considered in this regard.

For which reason, if you change foo to be const bool foo = true; then it will indeed compile.

Improve this answer

Comments

0

You should read about short-circuit evaluation. Simply put, it means: if you can find out the result of the logic statement at some steps (true or false), you can skip the rest expressions.

For example, in your case, you have 4 expressions:

  • I will call it x, y, z, t.
  • Your logic statement is: x && y && z && t.
  • If x is false, then the whole statement is false. So, we do not have to worry about the value of y, z, t. So, they will be skipped.

Now, in your situation:

purgeData = foo &&
            dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);

You can see, if foo is false, then data is unassigned because the rest expressions are not evaluated.

If you want to fix this problem, separate it into 2 statments:

purgeData = dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);
purgeData = purgeData && foo;

Or just simply move the expression assigning value to data to the first place, to make sure that it will be executed.

purgeData = dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data)
            && foo;

Or use & as Backwards_Dave's suggestion in his comment:

purgeData = foo &
            dataQueue.TryPeek(out data) &&
            data.attempt.startTime.AddMilliseconds(timeout) < DateTime.Now &&
            dataQueue.TryDequeue(out data);
Improve this answer

2 Comments

or use eagerly evaluated boolean operators like & instead of &&. & is not just for bitwise.
Why downvote? Please give a reason whenever you downvote someone.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.