Hello,
 
even if there's closed issue https://bugs.freepascal.org/view.php?id=34169 I would like to ask if it can be reconsidered.  
The subject is that SetLength now gives warning: Variable "dynamic array" of a managed type does not seem to be initialized in 3.3.1 and 3.1.1 while it doesn't give any warning in stable 3.0.4  
As a solution is recommended do declaration: var arr: array of widechar = (); But it means that initialization is in fact done twice. See assembler of following code:  
procedure TForm1.Button1Click(Sender: TObject);
var DA: array of widechar = ();
begin
  SetLength(DA, 10);
  writeln(DA[0]);
end; 
 
unit1.pas:30                              var DA: array of widechar = (); 00000000004696C9 488d1538536600           lea    0x665338(%rip),%rdx        # 0xacea08 <RTTI_$UNIT1_$$_def00000138> 00000000004696D0 488b3529536600           mov    0x665329(%rip),%rsi        # 0xacea00 <TC_$UNIT1$_$TFORM1_$_BUTTON1CLICK$TOBJECT_$$_defaultDA> 00000000004696D7 4889e7                   mov    %rsp,%rdi 00000000004696DA e8d199fcff               callq  0x4330b0 <fpc_dynarray_assign> unit1.pas:32                              SetLength(DA, 10); 00000000004696DF 48c74424680a000000       movq   $0xa,0x68(%rsp) 00000000004696E8 488d3519536600           lea    0x665319(%rip),%rsi        # 0xacea08 <RTTI_$UNIT1_$$_def00000138> 00000000004696EF 488d4c2468               lea    0x68(%rsp),%rcx 00000000004696F4 4889e7                   mov    %rsp,%rdi 00000000004696F7 ba01000000               mov    $0x1,%edx 00000000004696FC e8df99fcff               callq  0x4330e0 <fpc_dynarray_setlength> unit1.pas:33                              writeln(DA[0]); To avoid warning we now need four extra instructions incl. call to fpc_array_assign, which is obviously redundant.As an explanation is written: Setlength uses a var parameter. This is no different from other cases of var parameter usage.I disaggre here, because SetLength is not an usual procedure, it is fundamental part of compiler. Therefore it deserves some exception.That's why I ask for reconsidering and make it behave like in 3.0.4 or change declaration from "var" to "out" as it is done in getmem().Thanks, Vojtěch. 

Hello,
 
even if there's closed issue https://bugs.freepascal.org/view.php?id=34169 I would like to ask if it can be reconsidered.  
The subject is that SetLength now gives warning: Variable "dynamic array" of a managed type does not seem to be initialized in 3.3.1 and 3.1.1 while it doesn't give any warning in stable 3.0.4  
As a solution is recommended do declaration: var arr: array of widechar = (); But it means that initialization is in fact done twice. See assembler of following code:  
procedure TForm1.Button1Click(Sender: TObject);
var DA: array of widechar = ();
begin
  SetLength(DA, 10);
  writeln(DA[0]);
end; 
 
unit1.pas:30                              var DA: array of widechar = (); 00000000004696C9 488d1538536600           lea    0x665338(%rip),%rdx        # 0xacea08 <RTTI_$UNIT1_$$_def00000138> 00000000004696D0 488b3529536600           mov    0x665329(%rip),%rsi        # 0xacea00 <TC_$UNIT1$_$TFORM1_$_BUTTON1CLICK$TOBJECT_$$_defaultDA> 00000000004696D7 4889e7                   mov    %rsp,%rdi 00000000004696DA e8d199fcff               callq  0x4330b0 <fpc_dynarray_assign> unit1.pas:32                              SetLength(DA, 10); 00000000004696DF 48c74424680a000000       movq   $0xa,0x68(%rsp) 00000000004696E8 488d3519536600           lea    0x665319(%rip),%rsi        # 0xacea08 <RTTI_$UNIT1_$$_def00000138> 00000000004696EF 488d4c2468               lea    0x68(%rsp),%rcx 00000000004696F4 4889e7                   mov    %rsp,%rdi 00000000004696F7 ba01000000               mov    $0x1,%edx 00000000004696FC e8df99fcff               callq  0x4330e0 <fpc_dynarray_setlength> unit1.pas:33                              writeln(DA[0]); To avoid warning we now need four extra instructions incl. call to fpc_array_assign, which is obviously redundant.As an explanation is written: "Setlength uses a var parameter. This is no different from other cases of var parameter usage."I disaggre here, because SetLength is not an usual procedure, it is fundamental part of compiler. Therefore it deserves some exception.That's why I ask for reconsidering and make it behave like in 3.0.4 or change declaration from "var" to "out" as it is done in getmem().Thanks, Vojtěch. 

Hi,

seriously that is such a bullshit warning.

SetLength is called to initialize the variable, of course the variable is not initialized before. If it was initialized, we might not even need to call SetLength

Cheers,
Benito

The current FPC trunk issues only a hint for the SetLength().

Yuriy.

Surely the point of a hint/warning is to point out a potential issue that the programmer may wish to correct.

It should therefore be possible to change your code such a way that the hint/warning can be resolved so that it is no longer printed. This should not entail redundant or unnecessary code.

The Wiki still indicates that SetLength only allocates the space, but it does not initialized the memory. Although this is probably incorrect for "array of string" to prevent memory leaks.

I agree that SetLength should be a var parameter, as it can be used to reassign the size of a dynamic array, not just creating them. This must mean that the value passed is used, even for an assumably uninitialized array, so it must be initialized.

Readability of the code is not enhanced by doing an unnecessary static
initialization, you could do an equally ugly "pointer(var) := nil"

Would it not be simpler to have the compiler initialize all dynamic array variables to nil, like for string variables, which should prevent the uninitialized warning/hint without requiring special treatment for handling the SetLength function.

It does require that the compiler recognizes its own initialization of dynamic array variables.

Derek

1) Dynamic arrays are initialised with nil, but that is an implementation detail (required by the fact that they are reference counted: if they would contain random data, that would cause crashes)
2) Passing a reference-counted variable as a var-parameter without explicitly initialising it first triggers a hint in all cases. Suppressing this hint specifically for SetLength would require treating it specially.

There is a hint for such parameters even though the compiler knows they always contain a valid value, because valid in the sense of "won't crash the program" is not the same as "this is what the programmer intended". The only way for the programmer to indicate to the compiler what they want is by writing code.

The hints about the fact that managed parameters have not been explicitly initialised by the programmer are different from the hints you get for other types. This means you can selectively disable these hints in case you do not wish to be notified about the fact that the program contains no explicit initialisation of these variables. See the -vq and -vm command line parameters.

You don't need to typecast dynamic arrays to pointer to initialise them with nil. Simply "arr:=nil;" works. Ideally, the compiler would remove this extra initialisation if you add it before it got a different value, but it does not yet do that.

Jonas

This is meanwhile such a FAQ, maybe we should add it somewhere in the WIKI, Webpage, wherever ?

It's documented, but clearly not visibly enough.

Michael.

There is a hint for such parameters even though the compiler knows they always contain a valid value, because valid in the sense of "won't crash the program" is not the same as "this is what the programmer intended".

Are you baiting me, or was that accidental? ;-)

1) Dynamic arrays are initialised with nil, but that is an implementation detail

Is it, though? Global variables and instance fields are zero-filled, local variables as if the local variable block was a record passed to Initialize() (so, recursively zeroing managed fields). Although it is never spelled out, the Delphi manual heavily implies this.
I would guess most if not all pascal programmers wrote code that relies on that at some point.

@MvC: I'll come up with some examples and add it to the Portability page, since the different result variable initialization rules already are a bit of an issue.

I'm 100% serious about the above. Code should be explicit, not implicit. It's up to the compiler to remove unnecessary code based on what it knows is implicitly guaranteed. The programmer should only have to focus on the algorithms and on making their code as explicit/clear as possible. Removing initialisations based on the type of variables (which then may have to be re-added in case the type or usage changes) does not help with that.

In that sense, the optimisation capabilities of a compiler are quite important for the quality of the code that people write, because a better optimiser means that (some) people are less inclined to mangle their code and "tune" it to implementation details in order to coax the compiler into generating the code they want to see.

Yes, and that indeed would make it very hard to change any of that without breaking any code. The fact that people rely on implementation details does not make them any less of an implementation detail. In this context, "detail" does not mean "less significant fact or item" but "individual fact or item".

In general, the Turbo/Borland/Free Pascal community is very bad in terms of relying on implementation details. This is normal if you only have one dominant compiler. It's the same for individual companies (= mini-communities) that only use e.g. Visual C++ or GCC.

@MvC: I'll come up with some examples and add it to the Portability page, since the different result variable initialization rules already are a bit of an issue.

Since such code will also give wrong results in Delphi under certain circumstances (which most Delphi programmers probably don't know, because usually string result variables _are_ indeed nil there), this is a perfect illustration of my first point. Especially since unless you are clairvoyant, it is impossible to know that your function will never be used in the future in a way that will cause the result to be non-empty from the start.

Jonas

And they are all wrong in doing so. It's like spelling:

It's not because 99% of the population makes a mistake that it automagically becomes correct.

We can decide to adapt the (spelling) rules, but till that decision is made explicit, 99% of the population is actually making a mistake.

(
Cultural note:
For native english speakers the above may be somewhat farfetched: But in Dutch and French, spelling and grammar are not ruled by custom, but are strictly managed by official language committees...
)

@MvC: I'll come up with some examples and add it to the Portability page, since the different result variable initialization rules already are a bit of an issue.

I'm curious to see them, because all 'issues' reported here can be perfectly repeated (possibly with some modifications) in Delphi. The non-initialization of 'Result' has bitten me more than once in Delphi.

I can't explain things better than Jonas did. I intend to rework his arguments and introduce them in the language reference manual.

Michael.

Happy new year, everyone!

Michael Van Canneyt wrote

Cultural note: For native english speakers the above may be somewhat farfetched: But in Dutch and French, spelling and grammar are not ruled by custom, but are strictly managed by official language committees...

That is an extremely fitting example!

In my view, modern Pascal is ruled by custom, not by committee. The last time a committee wrote a Pascal standard, the result was technologically somewhat obsolete by the time it was published...
But that would be off-topic here.

I'm curious to see them, because all 'issues' reported here can be perfectly repeated (possibly with some modifications) in Delphi. The non-initialization of 'Result' has bitten me more than once in Delphi.

Aye. But it's rather rare in Delphi and very common in FPC, so not everyone might have encountered it (that's how I'm justifying to myself putting it on the Portability List instead of starting a Common Mistakes set ;-) ).

I can't explain things better than Jonas did. I intend to rework his arguments and introduce them in the language reference manual.

Feel free to use any of the examples here:
<http://wiki.freepascal.org/User:Martok/Portability_Issues#Managed_Variable_Intialization>

As you said, Delphi makes some of the same mistakes, but at least it doesn't emit warnings where the code-as-written is interpreted exactly-as-written, therefore, doing what the programmer told it to do in the first place. The problem I see is that these bogus warnings drown out the actual programmer errors signified by the same message.

First, I agree clearing dynarrays with :=nil is somewhat cleaner than Setlength(0). (Small issue: I only learned that this is even a thing from the compiler implementation of it when I was debugging #0031215! From the reactions of other Delphi developers I showed it to, it is _very_ uncommon.) Initializing to any number other than 0 can only be done with Setlength, and I'm going to be honest, this bit from compiler/symtable.pas looks just ridiculous.

      begin
        newbuiltdefderefs:=nil;
        builtdefderefs:=nil;
        builtsymderefs:=nil;
        setlength(builtdefderefs,deflist.count);
        setlength(newbuiltdefderefs,deflist.count);
        setlength(builtsymderefs,symlist.count);

The question then is: what is explicit? How does one tell apart "filler" code the compiler is supposed to remove and "relevant" code the compiler is not supposed to remove? Why even write code we hope will not be in the final binary?

Incidentally, this was the topic of a talk at this (well, last!) year's CCC [1]. Compilers doing overly clever optimisation are sometimes a real-world *problem*, not a solution. The compiler should at least tell the programmer about security relevant optimizations and offer ways to fix it if necessary.

[1] https://media.ccc.de/v/35c3-9788-memsad

Strongly agree.
Before I had to bury the project because of the whole enum portability fiasco, I rewrote large parts of the DEC routines in pure pascal, and it turned out that while being more readable and more portable, it was also *faster*, because FPC generates better instructions these days.

Op 2019-01-01 om 01:55 schreef Martok:

If you never do optimized builds in Delphi it is rare. But if you do optimized builds and have a lot of string code, you'll encounter it sooner or later.

They are not mistakes. It is by design.

In pascal, variables (and that includes "Result") are uninitialized. The compiler only makes sure the RTL does not crash on managed types, but you still must regard them as uninitialized and initialize them properly.

As long as you do that, there is no problem.

One can discuss whether the 'variables are uninitialized' rule must be abandoned, and declare that the compiler should initialize all variables with known values, but that is another discussion.

Michael.

On Jan 1, 2019, at 5:15 AM, Michael Van Canneyt wrote: One can discuss whether the 'variables are uninitialized' rule must be abandoned, and declare that the compiler should initialize all variables with known values, but that is another discussion.

I was going to say. Hasn’t it been discussed to at least make a compiler switch that initializes all variables? For non-realtime programs the performance penalty is irrelevant.

Regards,
Ryan Joseph

Hi,

The current FPC trunk issues only a hint for the SetLength().

Then it is a bullshit hint  ("bullshint")

If there ever is a fpc version that does not initialize them with nil, then it could show a warning. Till then it is pointless. Everything without an ISO standard is an implementation detail

2) Passing a reference-counted variable as a var-parameter without explicitly initialising it first triggers a hint in all cases. Suppressing this hint specifically for SetLength would require treating it specially.

SetLength has to be special, because we need it to initialize the array. There is no CreateArray function to initialize it with a somearray := CreateArray syntax, is there?

Even with the hint it is special, since at -O3 fpc warns about other functions, but not about SetLength anymore.

Indeed

The non-initialization of 'Result' has bitten me more than once in Delphi.

Me, too. Usually the solution was to add a call to SetLength.

That makes the bullshit hints especially bad.

There are so many hints about variables that are actually initialized that you cannot find the valid hints about places where the variable is actually uninitialized anymore.

Cheers,
Benito

Am Mi., 2. Jan. 2019, 17:05 hat Benito van der Zander geschrieben:

Even in Delphi situations can occur where a dynamic array Result variable is non-Nil already upon entry and then SetLength will simply manipulate that preexisting array instead of working on a new array. In most cases that probably won't have a real effect as the array is either set to empty anyway or all it's elements are initialized by the function. That doesn't change the fact however that the behavior *is* there and can potentially lead to hard to debug problems.

The only exception for the warning could possibly be SetLength(Result, 0) as that is the equivalent of "Result := Nil" or "Result := []" as 3.2.0 now supports.

Anything else can potentially be a bug lying in wait.

Regards,
Sven

Jonas Maebe wrote

1) Dynamic arrays are initialised with nil, but that is an implementation detail (required by the fact that they are reference counted: if they would contain random data, that would cause crashes)

Is it?

Because according to https://www.freepascal.org/docs-html/ref/refse24.html

Managed types are an exception to this rule: Managed types are always initialized: in general this means setting the reference count to zero, or setting the pointer value of the type to Nil.

It is documented, which makes it a documented feature?

Also https://www.freepascal.org/docs-html/ref/refse20.html#x50-680003.9 states explicitly that they are initialized to nil (and there is no restriction that local var are exempt from that).

Again, why is this not changed?

Clearly we can't change Delphi, but there is no real reason not to be better than them. And it *is* undocumented for them, so we'd still be compatible, just compatible-on-the-safe-side.

Document that managed variables are initialized (this also means calling operator Initialize) regardless of global/local, make sure that they always are (which is currently only missing the Result pseudovariable), and bam - problem solved.
Shouldn't even cause any extra code to be emitted since it's either already there or could be eliminated as a dead store.

Then we will get bug reports that FPC is slower than Delphi and generates more code, because it will have to allocate and initialise/finalise a separate temp instead of just passing an existing one without doing anything. Possibly followed by assigning this existing temp to its final destination after the function call, instead of directly passing the target as function result (for which we also have received bug reports in the past).

Jonas

The compiler cannot distinguish between the following cases:
* the programmer knows the compiler initialises it with empty/nil, and in this particular case that happens to be the exact value that is wanted
* the programmer forgot to initialise the variable with an initial value

They are separate hints/warnings so that they can be disabled separately by programmers who know they will always want an empty string/array as initial value in case of managed types.

Jonas

Jonas Maebe wrote

You don't need to typecast dynamic arrays to pointer to initialise them with nil. Simply "arr:=nil;" works. Ideally, the compiler would remove this extra initialisation if you add it before it got a different value, but it does not yet do that.

Something I just found out, while changing some old code: why does that not work?

  var
    u: UnicodeString = nil;
    // Incompatible types: got "Pointer" expected "UnicodeString"
    a: AnsiString = nil;
    // Illegal Expression
    c: TCharArray = '';
    // Syntax error, "(" expected but "const string" found

Simply using '' as an initializer for Strings and nil for arrays solves it, so it's no big deal. I'm just curious why Strings do not have the niln typeconv and why the TCharArray can't be initialized (but assigned at runtime) with ''. And why only the UnicodeString message actually relates what is wrong... ?

I'm fairly sure this was based on what Delphi allows and doesn't. Maybe it's because strings already had a primitive to initialise them to nil (and empty string) and arrays didn't (since array constants in code blocks did not exist at that time).

and why the TCharArray can't be initialized (but assigned at runtime) with ''.

'' is parsed as a string constant, which means internally it becomes an array of char. The compiler does not support compile-time conversions of (constant or not) regular arrays to dynamic arrays.

And why only the UnicodeString message actually relates what is wrong... ?

The CharArray error is also correct: array constants start with '('. I don't know why the ansistring error is less informative than the unicodestring one.

Jonas