4

The compiler shows me the following warning for the code below:

Warning: W1036 Variable 'Address' might not have been initialized

The code (an MVCE snippet based on real code):

function DoFoo(): Integer;
var
  i: Integer;
  Address, Bar: Cardinal;
begin
  for i := 1 to 5 do
  begin
    try
      Address := Hex2CardPos(IntToStr(i));
    except on EConvertError do
      continue;
    end;
    Bar := Address + 42;  // "Warning: Address might not have been initialized"
  end;
  Result := 42;
end;

As you can see, Address is either:

  1. Assigned to the Result of Hex2CardPos()
  2. Hex2CardPos() throws an error and the loop iteration is immediately skipped.

I tried to fix this by adding a useless Address := 0; to the beginning of the loop, but then the warning is just replaced with another:

Hint: H2077 Value assigned to 'Address' never used.

Is this a compiler bug or does the warning have substance?

DBedrenko
  • 4,871
  • 4
  • 38
  • 73
  • 2
    A `TryHex2CardPos` function would make your life a lot easier here – David Heffernan Mar 01 '16 at 15:27
  • @DavidHeffernan What would this function return in the case of bad input that triggers an `EConvertError`? It seems cleanest to let it raise an exception and let the handler handle it. – DBedrenko Mar 01 '16 at 15:51
  • 2
    If you are expecting to handle bad input data routinely, then a version that indicates success or failure by a boolean is generally cleaner than the version that uses exceptions. As a broad rule, if you want to wrap a single low level function call in an exception handler, the function is poorly designed. See `StrToInt` and `TryStrToInt`. – David Heffernan Mar 01 '16 at 16:30
  • 1
    It would return `False`, obviously, just like `TryStrToInt`. Use it like so: `if TryHex2CardPos(IntToStr(i), Address) then Bar := Address + 42`. This would make the warning go away, but mainly because the compiler's data-flow analysis ignores `var` parameters; it assumes that all `var` parameters are always assigned. – Rob Kennedy Mar 01 '16 at 16:30
  • 1
    I mean, the compiler is defective in issuing the warning and this is a good question. Although I do believe I've seen it before so it may be a dupe. (And there you go, I've found the dupe, and marked it accordingly.) But it's a good question. I just offer what I believe to be a better way to write your code. – David Heffernan Mar 01 '16 at 16:30

1 Answers1

8

The problem is in your code. "Bar" assignation has to be in the try except block because when an exception happens you dont want assign "Bar"

function DoFoo(): Integer;
var
  i: Integer;
  Address, Bar: Cardinal;
begin
  for i := 1 to 5 do
  begin
    try
      Address := Hex2CardPos(IntToStr(i));
      Bar := Address + 42;
    except on EConvertError do
      continue;
    end;
  end;
  Result := 42;
end;

Btw this code has a "H2077 Value assigned to 'Bar' never used" that's correct.

Agustin Seifert
  • 1,938
  • 1
  • 16
  • 29
  • Ha! So that's what it is. I followed the general rule which is to isolate as much as possible the code whose exception I intend to catch (i.e. the code in the `try` block). Thanks for the answer – DBedrenko Mar 01 '16 at 14:43
  • 3
    Just because you identified a permutation of the code that makes the warning go away doesn't mean the problem was in the code. The compiler is indeed wrong; you have identified a *workaround*. The compiler *should* be able to recognize in the original code that there is no path to the `Bar` assignment that skips the `Address` assignment. – Rob Kennedy Mar 01 '16 at 16:27
  • 1
    Agreed. The compiler is defective here. – David Heffernan Mar 01 '16 at 16:31