Jump to content
Mike Torrettinni

The Case of Delphi Const String Parameters

Recommended Posts

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?

Share this post


Link to post

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. 

  • Like 2
  • Thanks 1

Share this post


Link to post

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.

Share this post


Link to post

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

Share this post


Link to post
Posted (edited)

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 by Attila Kovacs
  • Like 1
  • Thanks 1
  • Haha 1

Share this post


Link to post
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 :classic_blink:

 

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

  • Like 1

Share this post


Link to post
Posted (edited)

@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 by Attila Kovacs

Share this post


Link to post
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.

Share this post


Link to post
Posted (edited)

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 by Rollo62

Share this post


Link to post

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?

Share this post


Link to post
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. 

  • Like 1

Share this post


Link to post

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 😉

Share this post


Link to post
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;

 

  • Like 2

Share this post


Link to post
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.

Share this post


Link to post
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 :classic_smile:

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 ?

Share this post


Link to post

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.

 

 

  • Like 1

Share this post


Link to post
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 ?

Share this post


Link to post
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.

Share this post


Link to post

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

  • Like 1

Share this post


Link to post
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 

Share this post


Link to post
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.

Share this post


Link to post
Posted (edited)
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 by Rollo62
  • Like 1

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×