The Case of Delphi Const String Parameters

[Mike Torrettinni 198]

I just read this post on Marco Cantu's blog: https://blog.marcocantu.com/blog/2020-december-Delphi-const-string-params.html

But I'm confused by his suggestions at the end: to not use const unless for performance case and that compiler will not change to detect such code. He even suggest the same for libraries. He does suggest how to change the code, but! he doesn't suggest to not write code like this at all.

 

It's almost like he is saying if you do any kind of strange string usage, just don't use const and all is OK. Of course, if you need to change the parameter, don't use const. But that is not what he is implying.

 

I looked into TField.GetValue but I don't see what he is referring to.

 

Does anybody know of a case similar to what he is showing that is better off not using const versus changing the code?

[David Heffernan 2345]

Nothing to see here. Carry on using const for string parameters. The issue is vanishingly rare. If you encounter the issue, fix it. Don't go changing existing code unless you are actually affected by the issue. 

[vfbb 285]

String works exactly like a bitmap (bitmap handle) from firemonkey.
When you create a string, through another, s2: = s1, this does not immediately create a copy of the string, but rather increases 1 refcount from a single original string, however when you change the string, internally it will check if the refcount is> 1 , it will create a copy, a new string with refcount 1, to make the changes, and not affect other places that are using that string. This was done to optimize, increase performance, as there are some manipulations of strings that in the end do not change the string, for example s2: = s1.PadLeft (0) will not create a copy, will return the string s1 with 1 more reference .

The use of const in the parameter is an optimizer as it avoids operations with the refcount. However Marco Cantu warns of a problem generated when using const in the method argument, because if inside it you indirectly change the original string (for example by calling an event within its method) and the refcount of that string is 1 (which possibly will be ), within its method the argument with const will become invalid.

[A.M. Hoornweg 144]

TMS FixInsight (a sourcecode analysis tool) flags non-const string parameters as bad practice if these strings are never written to.  

[Attila Kovacs 627]

I ran into this once

 

function x(const input: string; var output: string): boolean;

begun

  while we read input do

    alter output;

end;

 

if x(s,s) then;

 

except that it wasn't easy to track down, it was a one time "achievement" in my life. Yet.

 

 

given that, I consider every method having a const and var parameter of the same type as a time-bomb.

 

 

 

Edited January 2, 2021 by Attila Kovacs

[Guest]

21 minutes ago, Attila Kovacs said:

function x(const input: string; var output: string): boolean;

begun

  while we read input do

    alter output;

end;

 

if x(s,s) then;

My head is overheating trying to process the outcome of these six lines 

 

If i witness such code on my PC i would unplug the power and call an exorcist immediately.

[Attila Kovacs 627]

@Kas Ob. you don't have to be scenical, there are several methods out there with (const T; var T), and if you don't know the implementation detail you can run into it very quickly. The compiler won't give you any hints either.

Edited January 2, 2021 by Attila Kovacs

[Guest]

1 minute ago, Attila Kovacs said:

@Kas Ob. you don't have to be scenical, there are several methods out there with (const T; var T), and if you don't know the implementation detail you can run into it very quickly. The compiler won't give you any hints either.

In the past i did things, things i can't even repeat them now, last time saw one of them couldn't sleep for 2 nights.

 

I am just trying to be funny, i merely described the goosebumps that hit me remembering my old code.

[Rollo62 533]

I personally have not much problems with the const string,
but maybe it would be a more clean design always to use immutable strings instead (or together with) const ?

This would enforce an (unnecessary) string-copy usually, while the simple const may get away without any copy, in the best case.
I'm not afraid of additional copy too much, but I think also why should we do that, if there were only little const issues.

 

On the other hand, I assume there is no really immutable string possible in Delphi, so lets stay with const

(or I have to write my own complete string class maybe, with a lot of extra protection).

Edited January 2, 2021 by Rollo62

[timfrost 77]

We seem to be speaking here only about accidentally modifying the const string from inside the function.  And Marco's blog did not mention the word 'thread'.  But presumably another thread could cause the same address problem by modifying the string, and the problem would not arise if the function lacked the const and was therefore passed its own copy to read.   Or am I overcomplicating this?

[David Heffernan 2345]

31 minutes ago, timfrost said:

We seem to be speaking here only about accidentally modifying the const string from inside the function.  And Marco's blog did not mention the word 'thread'.  But presumably another thread could cause the same address problem by modifying the string, and the problem would not arise if the function lacked the const and was therefore passed its own copy to read.   Or am I overcomplicating this?

That would then be a race condition between the copy and the modification. That seems like the far bigger issue. 

[Mike Torrettinni 198]

Interesting how such a simple const vs non-const usage can be the cause of issues. I guess when you have a lot of lines of code in the method, it's easier to make mistake like that. Or even in such a short method, like in example, when you are not careful.

 

I guess, school is never out 😉

[A.M. Hoornweg 144]

On 1/3/2021 at 2:50 AM, Mike Torrettinni said:

Interesting how such a simple const vs non-const usage can be the cause of issues.

"Out" parameters are an even bigger can of worms.

I had refactored some code recently to use "OUT" parameters instead of VAR parameters (in order to more clearly document the intended use) and it had side effects that were hard to figure out. I thought my debugger had gone bananas.  Try single-stepping through this code and watch the values. 

 

In hindsight, the cause is clear, but I find the compiler should throw a warning if it encounters such a situation.  I now avoid OUT parameters.

 

 

procedure tform2.test(OUT somestring:String; Defaultvalue:String);
begin
  Somestring:=Defaultvalue;
end;


procedure TForm2.Button1Click(Sender: TObject);
var t:string;
begin
    t:='Testing 1-2-3';
    Test(t,t);
    Showmessage(t);
end;

 

[Mike Torrettinni 198]

1 hour ago, A.M. Hoornweg said:

Test(t,t);

Attila showed similar case above, with const, that is also a cause of issues. Perhaps there should be a check when using same variable more than once as parameter in same method.

[Lars Fosdal 1789]

Seems that https://tmssoftware.com/site/fixinsight.asp has no warning for this case, either.

[Rollo62 533]

Test(t,t);

Interesting, thats a nasty bug.

Again I'm happy that I prefer to use VAR over OUT, despite the fact that this is maybe philosophically a little sloppy

Since tthe DefaultValue is copied by value, I would not expect that behaviour.

I would call this a compiler bug, is there an RSP already ?

[A.M. Hoornweg 144]

I posted about it before in this forum and was told that it's intended behavior.   Admittedly, it's a corner case, but it did happen and cost me a lot of time to figure it out.  A compiler warning would have been nice.

 

 

[Rollo62 533]

1 minute ago, A.M. Hoornweg said:

..and was told that it's intended behavior...

Can you explain how that behaviour can be intended ?

Is there any argument why this is correct ?

[Lars Fosdal 1789]

It is not really a bug. The docs for the out parameter clearly state that the out variable contents will be discarded. 
The problem lies in that there is no warning that the value assigned will be discarded the before of the call, and not in the call.

 

http://docwiki.embarcadero.com/RADStudio/Sydney/en/Parameters_(Delphi)#Out_Parameters

[A.M. Hoornweg 144]

42 minutes ago, Rollo62 said:

Is there any argument why this is correct ?

"Out" parameters of managed types (such as strings, interfaces, dynamic arrays) are initialized before a method is called. 

 

The problem only arises because multiple parameters in the method reference the same string.  The behavior is correct but very counter-intuitive. It would behave more intuitively if the compiler would issue a UniqueString() for the second/identical parameter.

[Mike Torrettinni 198]

I see that PascalExpert has a check for this:

 

 

[Lars Fosdal 1789]

Looks like it is a check for passing the same param twice, not necessarily a check for the out param thing.
 

[David Heffernan 2345]

11 minutes ago, Lars Fosdal said:

Looks like it is a check for passing the same param twice, not necessarily a check for the out param thing.
 

If the tool was clever it would only object when the parameters were both passed by reference 

[Mike Torrettinni 198]

19 minutes ago, Lars Fosdal said:

Looks like it is a check for passing the same param twice, not necessarily a check for the out param thing.
 

True. Not perfect, but if you have only a few of these reported you can quickly manually check for issues and then disable the check.

Probably that's why FixInsight doesn't have this check, because it's more of a hint then warning or error.

[Rollo62 533]

3 hours ago, A.M. Hoornweg said:

"Out" parameters of managed types (such as strings, interfaces, dynamic arrays) are initialized before a method is called. 

 

The problem only arises because multiple parameters in the method reference the same string.  The behavior is correct but very counter-intuitive. It would behave more intuitively if the compiler would issue a UniqueString() for the second/identical parameter.

I would disagree to that, because a compiler should understand that by ref and by value of the same value is requested,

and should process the by value first, because the initialization of OUT should be considered "inside" the function, not from the caller.

http://docwiki.embarcadero.com/RADStudio/Sydney/en/Parameters_(Delphi)#Out_Parameters

Quote

...  the initial value of the referenced variable is discarded by the routine it is passed to ..

 

So the compiler could process all parameters before he touches any OUT parameter.

An OUT variable should be IMHO considered undefined, until "inside" the function some value is assigned to.
 

At least this is what I would expect here, and it should be easy for a compiler to detect such situation.

 

Edited January 4, 2021 by Rollo62