Bug in Delphi string behavior?

[A.M. Hoornweg 144]

Hello all,

 

I think I've discovered an anomaly in Delphi string behavior. Consider the following code:

procedure TForm1.Test(const Defaultvalue: string; out Resultvalue: string);
begin
  Resultvalue:=AnsiUpperCase(Defaultvalue);
  ShowMessage(Defaultvalue+'.'+Resultvalue);
end;

procedure TForm1.Button1Click(Sender: TObject);
var s:string;
begin
   s:='default';
   Test(s,s);
end;

 

The output produces just a dot, which makes no sense to me.

 

 

[Lars Fosdal 1792]

procedure TForm1.AnsiUpperTest;
var s, o:string;
begin
   s:='default';
   AnsiUpperTestConvert(s,o);
end;

Adding a separate output variable helps.

Can it be that the const DefaultValue is cleared by the out reference?

 

Alternatives: Use a var param or a function result.

[Attila Kovacs 629]

High five!

 

 

[A.M. Hoornweg 144]

I suspect that it may do so.  The "const" makes no difference. Using "VAR" instead of "OUT" also fixes the issue. 

 

I am quite alarmed by this issue because recently, whilst refactoring, I started replacing a lot of "VAR" parameters by "OUT" in order to make it more concise to a reader how the parameters are affected.  But now it appears that "OUT" is dangerous if you pass the same parameter multiple times.

Edited January 9, 2020 by A.M. Hoornweg

[Lars Fosdal 1792]

It is actually documented: http://docwiki.embarcadero.com/RADStudio/Rio/en/Parameters_(Delphi)#Out_Parameters

Quote

An out parameter, like a variable parameter, is passed by reference. With an out parameter, however, the initial value of the referenced variable is discarded by the routine it is passed to. The out parameter is for output only; that is, it tells the function or procedure where to store output, but does not provide any input.

Hence, since the const argument contains the same reference...

[A.M. Hoornweg 144]

2 minutes ago, Lars Fosdal said:

It is actually documented: http://docwiki.embarcadero.com/RADStudio/Rio/en/Parameters_(Delphi)#Out_Parameters

Hence, since the const argument contains the same reference...

Omitting the "const" changes nothing though. Delphi still passes it by reference even though it looks like a value parameter.  

 

[Lars Fosdal 1792]

Would it make sense to have a compiler hint for referencing initialized variables with an out parameter?

[A.M. Hoornweg 144]

28 minutes ago, Lars Fosdal said:

Would it make sense to have a compiler hint for referencing initialized variables with an out parameter?

IMHO the compiler should throw an error if the same parameter is passed multiple times to the method and one of them is an OUT parameter.

 

 

 

 

 

 

 

 

[Lars Fosdal 1792]

QP it, perhaps?

[Anders Melander 1782]

46 minutes ago, Lars Fosdal said:

QP it, perhaps?

Why? It's a programmer error. Not a bug.

[Lars Fosdal 1792]

Not the programming error - the possible need for a hint or a warning that helps a programmer avoid the error.

[A.M. Hoornweg 144]

Besides, the problem is not consistent, it only occurs with managed types.  For example, it does not occur with WideString. 

[Stefan Glienke 2002]

Sounds like an idea for a new rule to add for FixInsight and alike

[Remy Lebeau 1393]

On 1/9/2020 at 5:09 AM, A.M. Hoornweg said:

Besides, the problem is not consistent, it only occurs with managed types.  For example, it does not occur with WideString. 

It should, as WideString is a managed type.  It would be more accurate to say that it does not occur for non-managed types, like integers, etc.

[A.M. Hoornweg 144]

There is one thing I don't understand though.

 

In the example below, shouldn't the string which is held by variable S have a reference count of at least 2 when procedure Test is called? 

One held by S itself, one by Defaultvalue? 

 

Normally the compiler should perform a Uniquestring whenever something is written to a string with refcount >1,  or?

 

 

procedure TForm1.Test(Defaultvalue: string; out Resultvalue: string);
begin
  Resultvalue:=AnsiUpperCase(Defaultvalue);
  ShowMessage(Defaultvalue+'.'+Resultvalue);
end;

procedure TForm1.Button1Click(Sender: TObject);
var s:string;
begin
   s:='default';
   Test(s,s);
end;

 

[A.M. Hoornweg 144]

13 hours ago, Remy Lebeau said:

It should, as WideString is a managed type.  It would be more accurate to say that it does not occur for non-managed types, like integers, etc.

I meant, the allocated memory in a widestring isn't managed by the Delphi heap but by Windows. 

[Stefan Glienke 2002]

2 hours ago, A.M. Hoornweg said:

There is one thing I don't understand though.

 

In the example below, shouldn't the string which is held by variable S have a reference count of at least 2 when procedure Test is called? 

One held by S itself, one by Defaultvalue?

The refCount is -1 because its a reference to a string literal - UStrClr being called because passing to out does not deallocate but still put nil into the passed variable.

 

And fwiw the empty string also happens for WideString because then WStrClr is being called before passing as out. And while we are at it - yes AnsiString as well - LStrClr in that case.

Edited January 13, 2020 by Stefan Glienke

[A.M. Hoornweg 144]

20 hours ago, Stefan Glienke said:

The refCount is -1 because its a reference to a string literal - UStrClr being called because passing to out does not deallocate but still put nil into the passed variable.

 

And fwiw the empty string also happens for WideString because then WStrClr is being called before passing as out. And while we are at it - yes AnsiString as well - LStrClr in that case.

Strange, when I first noticed the problem it went away 100% after I replaced string with widestring.  But somehow I can't reproduce that anymore. Weird!

[Remy Lebeau 1393]

On 1/13/2020 at 12:46 AM, A.M. Hoornweg said:

I meant, the allocated memory in a widestring isn't managed by the Delphi heap but by Windows. 

Doesn't matter.  The Delphi compiler still manages WideString instances for you.  It knows to use the Windows memory manager instead of the RTL memory manager for allocating and freeing WideString data.  Doesn't make WideString any less of a managed type compared to other string types.