[fpc-devel] Wrong docs: not initialized global variables

[Ondrej Pokorny]

On 03.07.2018 23:09, Florian Klämpfl wrote:
> Am 03.07.2018 um 22:57 schrieb Ondrej Pokorny:
>> On 03.07.2018 22:08, Florian Klämpfl wrote:
>>> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>>>> On 03.07.2018 20:54, Florian Klämpfl wrote:
>>>> program Project1;
>>>> type
>>>>    TMyArray = array[0..10] of Integer;
>>>>    procedure Init2(var A: TMyArray);
>>>>    begin
>>>>      A[2] := 1;
>>>>    end;
>>>> var
>>>>    A: TMyArray;
>>>> begin
>>>>    Init2(A);
>>>> end.
>>>>
>>>> should not emit the warning because A is not being read in Init2.
>>>
>>> (please note it throws only a hint in this case as A *is* 
>>> initialized as it is a global variable).
>>>
>>> FPC has no global data flow analysis (and I do not plan to do so 
>>> yet), so this is warning is currently far out of reach.
>>
>> Doing it for SetLength/FillChar would be a good start.
>
> For me: Too much effort for very little gain. Global data flow 
> analysis in general is really a difficult topic and its advantages are 
> oversee-able imo.

Although being a little bit off-topic, IMO it's not that hard and the 
tools are in FPC already now. You have analysis for uninitialized local 
variable already. Use the same approach for the var argument and store 
the result as an argument flag:

procedure ProcA(var A: Integer);
var
   AA: Integer;
begin
   AA := AA + 1; // warning here
   A := A + 1; // same situation as with AA: warning for AA -> set the 
"var A" flag to "data in" + "data out"
end;

procedure ProcB(var B: Integer);
var
   BB: Integer;
begin
   BB := 1; // no warning here
   B := 1; // same situation as with BB: no warning for BB -> set the 
"var B" flag to "data not in" + "data out"
end;

procedure Test;
var
   X, Y: Integer;
begin
   ProcA(X); // << warning here because ProcA.A flag is "data in"
   ProcB(Y); // << no warning here because ProcB.B flag is "data not in"
end;

Then you should handle the "data not out"/"data out" flag as well. Yes, 
probably this analysis is an overkill and it only slows down the 
compiler with little advantage.

Such an analysis is not needed for SetLength because the flag "data not 
in"+"data not out" is there per definition. I agree about FillChar - I 
forgot it is a normal routine and not an intrinsic, so no luck with an 
assumption here.


> The compiler is ~400k lines and I had to fix maybe 40 locations, so I 
> wouldn't call this significantly and there are situations where it 
> matters, see the result case.

If you refer to:

function DoSomething (len: Integer): Vector;
begin
   SetLength(Result, len);
end;

IMO SetLength should be either invisible for variable initialization 
warning detection (because "data not in"+"data not out") or it should be 
a trigger for an uninitialized warning in subsequent code. (Probably the 
former case is easier and better.)

In both cases I described in the last paragraph, you should not emit a 
warning at SetLength call but you should emit a warning as if the 
routine was empty:
function DoSomething (len: Integer): Vector;
begin
end;
 >>> project1.lpr(6,10) Warning: Function result does not seem to be set

But yes, I agree that emitting a warning at SetLength is even easier. So 
yes, you are right. Better to show a warning at SetLength than not 
showing it at all.

Ondrej