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

[Florian Klämpfl]

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.

> 
> 
>>> IMPORTANT: the compiler knows that SetLength and FillChar don't read/use the data so no warning is appropriate there. 
>>
>> SetLength does somehow :), as the examples showed.
> 
> Not really. It keeps the "read/used" data within the variable - the possibly uninitialized data is not written or used 
> outside the variable itself.

?

> 
> 
>>> What you achieved with SetLength and FillChar emitting a warning is that you made the compiler more stupid instead of 
>>> making it more clever.
>>
>> The compiler has not to be clever, it has to follow rules. And the rules which need to be followed is the meaning of 
>> the var parameter. It means: data in/data out.
> 
> It's hard to argue like this with all the different hints/warnings/notes being emitted on the notification side and 
> different optimizations being done on the compiler side. To me it looks more like this: "the compiler may be clever only 
> when we want it to be".
> 
> ---
> 
> Probably I am the only one who thinks that the code below is ridiculous...
> 
>      procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p: pchar; len: longint);
>        var
>          s: ansistring;
>        begin
>          MaybeAddLinePrefix;
>          s:=''; // <<< ????
>          setlength(s,len);
>          move(p^,s[1],len);
>          AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
>        end;
> 
> ---
> 
> I have split feelings: I really like to get the warning/hint at places where it makes sense so I don't want to disable 
> it globally. But the amount of false positives increased significantly :/

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.