Use of unassigned local variable?

[Laura T.]

In the following example, c# 2.0 compiler says that a3 and ret are used
before assigned.
as far as I can see, definite assignment is made.

If I add

finally
{
ret = true;
a3 = "b3";
}

it compiles OK.

------ example -------
using System;
using System.Collections.Generic;
using System.Text;

namespace Scope
{
class Program
{
static void Main(string[] args)
{
string test;
Try(out test);
}

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}

ret = true;
}
catch (Exception)
{
a3 = "sa";
}

Hello = a3;

return ret;
}

}
}

 

[Peter Bradley]

Because there is a path through the method where the variable ret is not
assigned (bad form to call a method Try(), by the way).

If an exception is raised in the first try{} block, execution goes to the
catch block, where a3 is assigned. Execution then falls through to the code
after the catch and you try to return ret before it's been assigned.

Can't be bothered to look for a3.

A finally{} block is always executed - so that's bound to fix it. You could
always cure it by initialising your variables, of course, either with
initialisers or in the constructor.

HTH


Peter

 

[Laura T.]

Thanks Peter,

in the catch block both ret and a3 are assigned. They have the same flow.
Only a3 is shown as unassigned, ret not.
I still think there is something strange. IF I change the function (yes, bad
choice for the name) to this:

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}
ret = true;
}
catch (Exception)
{
ret = true;
a3 = "sa";
}

Hello = a3;

return ret ==true;
}

a3 is still unassigned to the compiler. IMHO both ret and a3 are assigned
definetly (it has catch all), and in any case, as they both have the same
flow, I'd at least expect that it would complain on both cases. Ok, there is
a ret=true, but it does not get executed if i.tostring() would throw. If I
comment it out, then compiler says that both ret and a3 are unassigned. If I
add a3="HELLO"; after ret=true; I get no errors.

Because there is a path through the method where the variable ret is not
assigned (bad form to call a method Try(), by the way).

If an exception is raised in the first try{} block, execution goes to the
catch block, where a3 is assigned. Execution then falls through to the
code after the catch and you try to return ret before it's been assigned.

Can't be bothered to look for a3.

A finally{} block is always executed - so that's bound to fix it. You
could always cure it by initialising your variables, of course, either
with initialisers or in the constructor.

HTH


Peter

In the following example, c# 2.0 compiler says that a3 and ret are used
before assigned.
as far as I can see, definite assignment is made.

If I add

finally
{
ret = true;
a3 = "b3";
}

it compiles OK.

------ example -------
using System;
using System.Collections.Generic;
using System.Text;

namespace Scope
{
class Program
{
static void Main(string[] args)
{
string test;
Try(out test);
}

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}

ret = true;
}
catch (Exception)
{
a3 = "sa";
}

Hello = a3;

return ret;
}

}
}

 

[Wilfried Mestdagh]

Hi Laura,

Indeed, a3 is always assigned in your code as well is ret. The
difference is that you assign a3 in the for loop. Possible the compiler
does not "see" it There are lots of situations like this, but in
dotNet it is not a warning, but an error. In this case you could assign
null or so when declaring it to make the compiler happy

--
rgds, Wilfried [MapPoint MVP]
http://www.mestdagh.biz

Thanks Peter,

in the catch block both ret and a3 are assigned. They have the same flow.
Only a3 is shown as unassigned, ret not.
I still think there is something strange. IF I change the function (yes, bad
choice for the name) to this:

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}
ret = true;
}
catch (Exception)
{
ret = true;
a3 = "sa";
}

Hello = a3;

return ret ==true;
}

a3 is still unassigned to the compiler. IMHO both ret and a3 are assigned
definetly (it has catch all), and in any case, as they both have the same
flow, I'd at least expect that it would complain on both cases. Ok, there is
a ret=true, but it does not get executed if i.tostring() would throw. If I
comment it out, then compiler says that both ret and a3 are unassigned. If I
add a3="HELLO"; after ret=true; I get no errors.

Because there is a path through the method where the variable ret is not
assigned (bad form to call a method Try(), by the way).

If an exception is raised in the first try{} block, execution goes to the
catch block, where a3 is assigned. Execution then falls through to the
code after the catch and you try to return ret before it's been assigned.

Can't be bothered to look for a3.

A finally{} block is always executed - so that's bound to fix it. You
could always cure it by initialising your variables, of course, either
with initialisers or in the constructor.

HTH


Peter

In the following example, c# 2.0 compiler says that a3 and ret are used
before assigned.
as far as I can see, definite assignment is made.

If I add

finally
{
ret = true;
a3 = "b3";
}

it compiles OK.

------ example -------
using System;
using System.Collections.Generic;
using System.Text;

namespace Scope
{
class Program
{
static void Main(string[] args)
{
string test;
Try(out test);
}

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}

ret = true;
}
catch (Exception)
{
a3 = "sa";
}

Hello = a3;

return ret;
}

}
}

 

[Jon Skeet [C# MVP]]

a3 is still unassigned to the compiler. IMHO both ret and a3 are assigned
definetly (it has catch all), and in any case, as they both have the same
flow, I'd at least expect that it would complain on both cases. Ok, there is
a ret=true, but it does not get executed if i.tostring() would throw. If I
comment it out, then compiler says that both ret and a3 are unassigned. If I
add a3="HELLO"; after ret=true; I get no errors.

ret is definitely assigned at the end of the try block - a3 isn't. You
see, the compiler doesn't see the difference between:

for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}

and

for (int i = 10; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}

Note how it will never enter the main part of the block in the second
example. The compiler doesn't check that the condition of the for loop
is always true for the first iteration, and thus make a1-a3 definitely
assigned because of it.

 

[Chris Dunaway]

Thanks Peter,

in the catch block both ret and a3 are assigned. They have the same flow.
Only a3 is shown as unassigned, ret not.
I still think there is something strange. IF I change the function (yes, bad
choice for the name) to this:

private static bool Try(out string Hello)
{
bool ret;
string a1, a2, a3;

try
{
for (int i = 0; i < 3; i++)
{
a1 = "1";
a2 = i.ToString() + a1;
a3 = "HELLO";
}
ret = true;
}
catch (Exception)
{
ret = true;
a3 = "sa";
}

Hello = a3;

return ret ==true;
}

a3 is still unassigned to the compiler. IMHO both ret and a3 are assigned
definetly (it has catch all), and in any case, as they both have the same

No, because the compiler can't be sure that the code inside the for
loop will be executed so there is a chance that you might exit the try
block without assigning a3. Suppose your for loop used a variable as
the upper limit instead of a number:

for (int i = 0; i < SomeVar; i++)

Since the compiler does not know the value of SomeVar at compile time,
it can't be sure that the for loop will be entered so that a3 can be
assigned.

I think this is why it is complaining, because there is a path through
the code in which it is possible for a3 to remain unassigned when it
reaches the line where you are using its value to assign to the Hello
variable.

 

[Laura T.]

Hi Chris,

Thanks for your comments.

for (int i = 0; i < SomeVar; i++)

Since the compiler does not know the value of SomeVar at compile time,
it can't be sure that the for loop will be entered so that a3 can be
assigned.

In this case, I would agree, because SomeVar is not constant. But my example
loop, for (int i = 0; i < 3; i++), is explicitly local and "constant".
Compiler should have no trouble teven to unroll the loop since it has no
other constraints than the constant 3.
When the compiler checks that the condition is bool it could also do rest of
the analysis, no?

LT

 

[Marc Gravell]

Technically, the compiler possibly could - but it would not meet the
very specific rules (on definite assignment) in the C# specification,
so you'd probably need to change that first...

This unrolling would only work for very simple examples. Personally, I
like that it is consistent - i.e. the rules are the same for both
complex and simple scenarios. I am human, and I am fallible: I can
only remember so much. If the current compiler behaviour reduces the
number of special cases I need to worry about, then I say "good for
it".

Marc

 

[Laura T.]

Jon,

I don't see any rationale for the compiler not checking the loop. As it
checks the condition to resolve it as bool it could check if the body is
reacheable. The c++ compiler compiles it without complaining.

As your for (int i = 10; i < 3; i++), shouldn't the compiler give a warning
"Unreachable code detected". i has local scope so it cannot be changed
anyway outside it's scope.

LT

 

[Laura T.]

IMHO it is not so trivial and uncommone case. It can lead to some good
optimizations.
A quite uncommon case is when the loop is changed to for (int i = 10; i < 3;
i++) so it's clearly not excutable and the expression has no side effects
since its local scope, the compiler/JIT still generate code for it.

for (int i = 10; i < 3; i++)
00000066 inc dword ptr [ebp-28h]
00000069 cmp dword ptr [ebp-28h],3
0000006d jl 0000003F

For example the C++ compiler for example here skips all the code, because it
is unreacheable (no warning though).
Ok, it's not the best example but just a curiosity.

 

[Marc Gravell]

OK, the "i=10;i<3" case is a bit different, and it would be an
interesting possiblity for it to observe this as an "unreachable"
warning. I'd be all for that.

However, re the unassigned issue - it really is following the rules,
specifically 12.3.3.7 and 12.3.3.9 of ECMA 334. This boils down to: if
"for-initializer" or "for-condition" definitely assign the value, then
it is definitely assigned. Neither "for-iterator" nor
"embedded-statement" are tested for this purpose, as *in the general
case* neither is guaranteed to execute.

The problem about introducing this as compiler logic (without changing
the spec) is that you then start getting other supposedly compiliant
compilers that can't compile the same code. At best you might do it
via an "non-compliant optimisations" switch... but I don't think I'd
ever turn it on.

By the time you have considered all the permutations (e.g. exceptions
(OOM for instance) or captured variables) you end up with a very
limited subset of cases when you could legitimately apply it... It
would be one heck of an "unless this, that, the other and all-of-these
do these" special case for the language specification, and I can't
really see the benefit of doing it! It works and is simple to follow,
so why complicate it unless it is made *significantly* better?

Personally, I'd rather "fix" this with an "= null" (inline
initialiser) for the a3 case (ignoring the obvious "set to the same
thing 3 times" wossit), and setting to false in the exception handler
for the "ret" case.

Marc

 

[Laura T.]

Thanks for your insight. I was thinking of the following rule from the ecma:

"
12.3.3.1 General rules for statements

The definite assignment state of v at the end point of a block, checked,
unchecked, if, while, do,
for, foreach, lock, using, or switch statement is determined by checking the
definite assignment
state of v on all control flow transfers that target the end point of that
statement. If v is definitely
assigned on all such control flow transfers, then v is definitely assigned
at the end point of the statement.
Otherwise, v is not definitely assigned at the end point of the statement.
The set of possible control flow
transfers is determined in the same way as for checking statement
reachability (§15.1).
"

More precisely "if v is definitely assigned on all such control flow
transfers, then v is definitely assigned at the end point of the statement."

I'm also all for simple definitive rules for c# (unlike c++) that make
writing code a lot easier. I was just wondering what's up here.

Thanks

LT

 

[Marc Gravell]

I was thinking of the following rule from the ecma

Me too, which is precisly why the cited code (OP) fails. It is my
understanding (reasonably) that the interpretation of each control
flow transfer is as defined in the adjacent sections, and hence the
definite assignment state (for v) for the flow transfer in question
(for [12.3.3.9], interpreted as while [12.3.3.7]) says that a3 is
*not* definitely assigned.

Marc

 

[Jon Skeet [C# MVP]]

I don't see any rationale for the compiler not checking the loop.

The language would be significantly more complicated, IMO.
Detecting the result of the assignment and the fact that the condition
would always be
true the first time would be hard to specify exactly, I suspect. You
could get some simple situations - eg creation of a local numeric
variable and straight comparison in the condition - but then we'd get
questions about why it detected that but not other conditions.

The language designers decided to keep the language simple in this
respect - and I for one think it was the right decision. The rules
about definite assignment are complicated (and tedious) enough
already, without adding extra nasties in.

As it checks the condition to resolve it as bool it could check if the body is
reacheable. The c++ compiler compiles it without complaining.

Does the C++ compiler complain about unassigned local variables at
all? (I've no idea.)
Furthermore, do *all* C++ compilers not complain about it? One of the
things about C# is that what is legal and what is illegal is very
tightly defined.

As your for (int i = 10; i < 3; i++), shouldn't the compiler give a warning
"Unreachable code detected". i has local scope so it cannot be changed
anyway outside it's scope.

No, because it doesn't try to check the condition against the
assignment.

Jon

 

[Laura T.]

Thanks Jon,

I get your point, and tos ome extent I do agree. The rules of c++ are making
the language more and more harder to understand.
It's something to avoid. But still, I admit, this rule of definite
assignment is a bit confusing, for me. But I can live with it, as I now
understand it.
And now I must fix my static analysis tool to catch this.

LT.

btw, yes, most C++ do catch at least some bad habits: cscope.cpp(38) :
warning C4700: uninitialized local variable 'a3' used, but still they lend
you the famous rope.

 

[Peter Duniho]

Does the C++ compiler complain about unassigned local variables at
all? (I've no idea.)

Yes, you can turn that warning on (and of course, you can set the compiler
to treat the warning as an error if you like).

Personally, I think it's well and good to keep things simple. However,
the problem here is that the unassigned variable is always an error but
the compiler incorrectly detects provably-assigned situations as
unassigned. I understand keeping the language definition simple, but in
this case the very act of changing the code to get rid of the error can
lead to the very bug that the error would otherwise be protecting
against. That is, by requiring the programmer to provide a default value,
if the code is changed later so that the variable is not *properly*
initialized (there may in fact be no appropriate "default" value, even
though the compiler insists on it), then what is essentially an
"uninitialized variable" bug can easily creep in, even as the compiler has
an error that's supposed to detect and prevent such a bug.

We've had this discussion before, and I admit you may not find my point of
view compelling. But personally, it's my opinion that if a compiler
cannot definitively determine a situation, it has no business emitting
errors regarding that situation. It's fine for the compiler author and/or
language designer to say that the compiler shouldn't be required to
analyze loop conditions when deciding whether a variable is initialized or
not, but if that's the choice that's made, the compiler shouldn't be
making the uninitialized variable condition an error.

Personally, I don't think it would be all that hard for the compiler to
deal with invariant loop boundaries and properly determine whether the
loop (or other block of code, for that matter) is always executed or not.
I also don't think it would be all that confusing to the programmer for
some loops to be flagged as "always entered" or "never entered" while
other loops are treated as "don't know" (and thus, for the uninitialized
variable situation treated as "never entered", that being the worst-case
scenario).

Pete

 

[Peter Duniho]

[...] If the current compiler behaviour reduces the
number of special cases I need to worry about, then I say "good for
it".

Except that in doing so, the compiler creates a situation where you wind
up being forced to write code that negates the whole point of the error in
the first place.

Pete

 

[Ravichandran J.V.]

Laura,

You are expecting the compiler to unroll whereas that would be
metaprogramming and the csc is just a normal compiler that generates a
simple C# program and not generate a metaprogram.

The "Use of unassigned variable " error is a good example that the csc
initiates good programming practice.

As the other posts have noted, it is because the assignment is within
the loop, the compiler cannot "pass" the Hello=a3; statement where the
visibility of the assignment to a3 is not the same as that of Hello's.

with regards,


J.V.Ravichandran
- http://www.geocities.com/
jvravichandran
- Or, just search on "J.V.Ravichandran"
at http://www.Google.com

 

[Laura T.]

I might not get your point, but I don't see anything "meta" in compiler
optimizations.
In the whole block of code, since all variables are local, the dataflow
diagram is very easy and linear, and most optimizing compilers would reduce
the whole block of code to Hello="HELLO"; since the final assignment is
invariant.
But as it seems that csc 2.0 does not analyze dataflow inside for/while
blocks, it cannot do it and thus it cannot eliminate common subexpressions
either (both a1="1"; and a3="HELLO"; are invariant and should be moved
outside the for block, thus eliminating the "use of unassigned variable", so
that the "ideal" block of code would be:

for (int i = 0; i < 3; i++)
{
a2 = i.ToString() + a1;
}

a1 = "1"; // invariant, move outside
a3 = "HELLO"; // invariant, move outside


Incidentally, if *I* do this, the compiler is happy as it can find the
definite assignment!).

Hope that someday csc will begin to do some optmizations that are unpratical
for JIT.

LT

 

[Laura T.]

I just had a discussion with my collegues, and they pointed out this:
If the c# compiler had an optimizer (now it just does some
microoptimizations), the compiler would write the code needed to make it
happy in my example:

for (int i = 0; i < 3; i++)
{
a1 = "1"; // move outside the loop
a2 = i.ToString() + a1;
a3 = "HELLO"; // move outside the loop
}

ret =true;

would become (cannot eliminate the loop but should move out invariants):

a1 = "1";

for (int i = 0; i < 3; i++)
{
a2 = i.ToString() + a1;
}

a3 = "HELLO"; // now we have conforming definitive
assignment written by the optimizer
// the same thing the compiler
expected me to do
ret =true;

It's nothing more than a curiosity.