I've been reading over some of the recent discussion about reference counting problems with const string parameters. I've done some experiments and I believe that the so-called const optimization is a serious flaw, not just a corner case of questionable legitimacy. I have some sample code I will show which should be quite scary. Additionally, this is a security vulnerability. It is also a quiet bug, because it may go undetected for a long time but randomly result in unreproducible crashes.

Consider:
* It does not affect just global variables but also object fields. While global variables are used rarely, object fields are used constantly in OOP.
* It does not affect just strings but also interfaces and dynamic arrays.

Of course, any compiler optimization is based on certain logic. The compiler can determine certain truths about the code at compile time and use those truths to avoid unnecessary operations. The basic logic behind const optimization is incorrect I believe, so I consider this a bug.

Forgive me if I am mistaken, and please offer corrections, but I believe the logic to be as follows, and that it is incorrect, as I will explain.

I think the logic for const optimization would go something like this:

1. Definition: A reference count is the number of variables that point to the same instance of something (strings, dynamic arrays, and interfaces).
2. By definition: The reference count will decrease whenever a variable stops pointing to that instance. This can occur if: (a) the variable goes out of scope, (b) the variable is reassigned to a different instance, or (c) the content of the variable is modified, thus triggering copy-on-write and creating a new instance, effectively triggering (b).
3. Definition (of const): Within the procedure a const parameter cannot be reassigned, it cannot be modified, and (of course) it does not go out of scope while the procedure is running.
4. Deduction from 2 and 3: During the time the procedure is running, the reference count will always be greater than or equal to the reference count when the procedure first began running.
5. Assertion (background knowledge): The reference count of the instance was greater than zero before it was passed to the procedure.
6. Deduction from 4 and 5: The reference count cannot become less than or equal to zero while the procedure is running.
7. Assertion (background knowledge): An instance's memory is deallocated only when its reference count is decremented to zero.
8. Deduction from 6 and 7: The instance cannot be freed while the procedure which received it as a const parameter is running.
9. Implication from 8: There is no need to increment and decrement the reference count upon entering and leaving the procedure.

I may have glossed over a couple details but I think this is sufficiently clear.

The logic is incorrect. The problem appears to be in step 4. The logic sort of conflates the notions of "instance" and "variable". While it is true that the variable, which is a const parameter, cannot be reassigned, it is NOT true that the instance itself cannot be modified. In fact other variables that refer to the instance can be changed, and the refcount can decrease.

We already know this; the reason I went through all the logic is because I want to emphasize that the "optimization" of not updating the refcount for const parameters is fundamentally wrong. It is not just a corner case or a slight oversight in the language design; it is a full-fledged bug. Going back to what I said earlier, we all recognize a compiler optimization is based on determining certain truths about the code at compile time. Well, the truth behind const optimization isn't true.

The question, then, is: how serious is it? A few things to consider:
* It affects all OOP, even if you're not using any global variables.
* It affects interfaces and dynamic arrays as well.
* It is a SECURITY vulnerability. This bug makes it possible to write to memory that has been deallocated and possibly reallocated to a different use. This is a serious security flaw.
* It is difficult, if not impossible, to detect. There are often enough other references to an instance that the refcount will not actually get down to zero. So the problem does not show up most of the time. But whether or not there are other references can depend on the logic of the program, user input, etc., and is essentially unpredictable.
* When it happens, it will be almost impossible to diagnose. Careful examination of the code will reveal nothing. Since the bug can be triggered rarely and essentially at random, careful testing and step-by-step debug tracing may still reveal nothing.
* Even when the memory is deallocated prematurely, it won't necessarily crash. Actually, the program will often continue running with the deallocated memory and appear to be working fine until that memory is allocated for something else. Thus when the program finally does crash, the original cause of the crash may have happened long ago and have no direct connection to the place where the fault finally occurred.
* While examples demonstrating the behavior tend to be simple, and so it may seem that the problem should have been obvious to the programmer, in real use the problem can be nested deep in the interactions of many layers of code.

It is impossible for a programmer to confidently write a correct program in Free Pascal and believe it will work.

I have prepared some sample code to demonstrate how easily this can happen without anyone noticing and how subtle it can be. Please give it a try:

http://vobarian.com/downloads/RefCountCrashDemo.zip

Be sure to read the _README.txt file for instructions on how to do the demo.

I also have sample code of crashing with global variables, interfaces, and dynamic arrays. If anyone would like to see it just let me know and I'll upload it.

The next question is: what can be done? Several schemes have been proposed to test for the error. However, it seems likely that some of these may impose more overhead than simply eliminating the original "optimization". The safest and cleanest thing to do is to remove the incorrect const optimization. However then we have to worry about performance. The performance impact depends entirely on the application. I have done several string manipulation performance benchmarks, which I can also share if anyone is interested. The brief results are (in arbitrary time units; lower is better):

Palindrome Testing:
166 no const
170 with const
const is 2.3% slower

Word Wrapping:
1318 no const
1297 with const
const is 1.6% faster

XML Processing (counting nodes & loading into TMemDataset, using customized XML libraries):
668 no const
575 with const
const is 13.9% faster

All three benchmark programs are almost entirely string manipulation. The results are somewhat inconclusive. These were brief tests, so more rigorous and well-thought-out testing would be needed to give an accurate impression. The palindrome test was consistently faster (I ran each test many times) without const, but only by a tiny bit. I can't figure out why. I probably overlooked something in the code. The XML processing takes a serious hit, but as with the palindrome testing, it was a quick test so I may have overlooked some cause of inefficiency. Of course XML is disgustingly inefficient anyway, but that's beside the point :)

One thing that a performance test must also take into account is whether there are other variables that cause a try-finally to be set up. Since the overhead appears to mostly be in the try-finally, for procedures where there would have to be one anyway, the use of const probably doesn't make much difference.

My knowledge of FPC internals is almost none, so I can't really offer a good suggestion. My best proposal would be to eliminate the "optimization" and come up with a much lighter form of exception handling to make up for the speed. I am skeptical of any attempt to keep the optimization but check for errors because I think there is a good chance that there will be some oversights, some cases nobody thought of, not to mention the fact that it increases complexity.

- Chad

Chad, I see your point, but you're exaggerating a little imho.

I tried this with Delphi 5, although the program lived, the string became unusable, contained gardage after, so probably Borland already had this issue many years ago. Don't get me wrong, but if you want to fool the compiler, you will always be able to do this since you're a human and it is just a program :) It's subjective on my side, but If it existed so many years in borland/inprise/embarcadero products and I can not remember my own bug fixes related to something like this, I consider your example more artificial than one might think. But I'm not representative here, and maybe others have other examples.

Also, can someone else test this on later delphis?

Max

....

I don't think it is a bug.

However It should be more prominently documented, I have found in neither of those places:
http://www.freepascal.org/docs-html/ref/refsu10.html#x33-360003.2.4 http://www.freepascal.org/docs-html/ref/refsu58.html#x135-14500011.4.4

A bug, to me is where the actual behaviour and the intended behaviour are different. However the behaviour observed is intended. (So yes, per definition a crash is only a bug, if the crash was not intended).

You can argue that this feature is not a good design decision. That is a different question. But anyway, this feature is already used, and therefore will cause lots of complains if removed.... Anyway, if you do not like the feature do not use it. ("const s : string", has no other effect but only the missing ref count => so by not using "const" in that case, you loose nothing, except that you get the refcount back)

In more detail:
"(const s: string)" is a declaration by the programmer, that the variable (the string) will not be is neither by
the procedure called, nor by any code outside the procedure, via any other reference to the data.
If the programmer sticks to what he declared, then it works, if not it crashes.

From this point of view it is no different to:
"SomePointerToRecord^.RecordMember". If there is NO "if SomePointerToRecord nil then " to protect it, then the programmer has declared that he has otherwise taken care that the pointer is valid at the time. If the pointer is nil (or otherwise invalid), then it will crash.

Sure the pointer issue is much easier to spot. But the point is, the programmer makes a statement what is to be expected. If this expectation is not fulfilled, it is a problem of the implementation, not the feature.

For all else:
- it is probably good to draw attention to it, because the majority of people seem to be unaware. Being unaware of how are feature works, and yet using that feature, has a big potential of going wrong. (but that is not a problem of the feature, only a problem of the incorrect usage)

- Request have been made:
A global switch to disable the optimization, for those who want not to use it.
A debug/sanity check (like range checks) would be good

- Request have been made: A global switch to disable the optimization, for those who want not to use it.

Nice :-)

DoDi

Max wrote

It's subjective on my side, but If it existed so many years in borland/inprise/embarcadero products and I can not remember my own bug fixes related to something like this, I consider your example more artificial than one might think.

I understand what you're saying, and I had the same thought myself. It does seem hard to believe that if this were really serious it would go unattended to for so long. So, I realize that it probably does not happen very often. But I for one cannot say I like using a compiler that "usually" works.

Keep in mind the main reason why we don't see this fail more often: Usually there are a lot of other variables referring to an instance. So, although the refcount is probably decremented incorrectly rather frequently, it is only rarely that we see it actually become a problem (when it reaches zero).

But I don't consider the fact that it is rare and /usually/ harmless to be an excuse for incorrect results.

I also don't think we should limit ourselves by settling for some of the mistakes that may have been made in the past in Delphi.

Martin wrote

I don't think it is a bug.

...

Is that true? I am not necessarily asserting that your statement is false; it could well be true. However I personally have not seen it documented so if anyone has a reference I would like to see it. As far as I have been able to tell from the docs of Delphi and FPC, the const declaration does NOT mean the programmer promises to not modify the instance; it means he promises to not modify the variable, which the compiler does indeed enforce. The variable and the instance are not the same. The variable is const but I have not seen documentation to suggest that the instance is supposed to be treated as const by the programmer. If I am incorrect then I will gladly accept whatever documentation you can direct me to. Also, as my demo shows, the programmer may not even know when an instance is being used as a const variable. In my demo, the programmer does not do a function call but rather assigns a property; it just so happens that behind the scenes the setter takes a const parameter.

Anyway, if you do not like the feature do not use it.

Unfortunately, that is not true. I don't have to use const in my own code, but I cannot control the fact that it is already widely used in other libraries, including those of FPC.

But anyway, this feature is already used, and therefore will cause lots of complains if removed.

I agree that if performance is significantly hurt there will be complaints. But I don't see how that negates the need for it to work correctly. What is needed is a solution that does not degrade the performance noticeably, though I realize that is easier said than done.

I just want to ask, after looking at the demo, is this not the least bit scary to anyone? I mean, you can make one very simple, tiny change and turn a perfectly working program into an immediate crash for no apparent reason. The secrets are buried in there. In a real world scenario you wouldn't be able to see it crash so easily. It may or may not crash, or it may just crash much later, or it may not crash but just be a security vulnerability waiting for someone to discover.

In this regard, I want to go back to something Max said: "I can not remember my own bug fixes related to something like this". I agree: me neither. But mysterious crashes do happen, and sometimes someone might "fix" them by changing some other things which are only tangentially related, i.e., they have a convenient side effect of hiding the bug by, for instance, affecting the reference counting by coincidence, as such interactions can be quite deep.

We also have to remember that *probably almost nobody* remembers fixing a bug related to this. That's because most people who come up against this bug probably have no clue what is happening and only by making some other changes do they coincidentally "fix" it. I propose that for a problem like this you cannot rely on reported incidents to determine how frequent it is.

If it helps, I can remember fixing this bug. It took two days because due to past mistakes I find it very difficult to convince myself that the bug is actually in the compiler and not in my code.

So yes, it is very scary to me and would like it to be fixed.

I did not submit a bug because I could not reproduce it without submitting the whole application. The solution was to remove the const from the parameter list.

Chad Berchek wrote

We also have to remember that *probably almost nobody* remembers fixing a bug related to this. That's because most people who come up against this bug probably have no clue what is happening and only by making some other changes do they coincidentally "fix" it. I propose that for a problem like this you cannot rely on reported incidents to determine how frequent it is.

Yesterday I have initiated a review of the (Delphi) code in one company I work for, to check for
this issue. So far, we have found two instances of procedures with non-const string parameters
with the comment to the effect of "do not change to const, or the code will break".
One of these instances is fairly recent, and the developer who made the change did remember
that it cost him more than a work-day to fix -- and that was after the crash report from the client.
The company is now considering outright ban on all const string parameters, similar to already implemented ban in const interface parameters.

I really think FPC should do better than this.
See also:
http://stackoverflow.com/questions/5851584/are-const-string-parameters-thread-safe http://chrisbensen.blogspot.com/2007/11/string-parameters-in-delphi.html?showComment=1196373240000#c7340129019635270394

Repeating my proposal from the bug-report
(http://bugs.freepascal.org/view.php?id=19605):
1) Introduce new optimization switch, e.g. {$OPTIMIZATION CONSTNOREF}, preferably locally-scoped.
2) Include that switch in UNCERTAIN group and disable it by default
3) In certain specific cases, enable the optimization anyway if it can be proved safe.
4) Enable this optimization *even for non-const parameters* in the same cases.

I hope the last two points can seriously reduce performance hit (or even gain some improvements) without compromising correctness.

From the top of my head, I can see following safe cases available

without cross-function analysis:
1) inlined procedures
2) leaf procedures
3) procedures where string parameters are not accessed after the first call to another function

The last case, while sounding artificial, can be actually quite common in the setters
for string properties:

procedure TMyObj.SetProp(AValue: String);
begin
  if FProp = AValue then exit;
  FProp := AValue;
  MyObjChanged;
end;

Well, I have pointed out myself,in my mail, that it probably needs more documentation. I do not know if it is documented or not.

But it is the answer, I have gotten several times from developers in the FPC team. So for all I know it is the intended behaviour. At least intended in FPC (and apparently either intended or at least implemented in Delphi). So if there is no documentation for it, then it would appear a problem of documentation, rather than a bug in the compiler (Again all based on the statements I was given)

Well yes. But that is not a problem limited to the "const string param". Any code you use in your app, can contain any kind of bug.

Very true, in fact the whole issue has only recently been discovered in the LCL too (as a bug in the LCL)

I would gop as far as to say, every time someone use a "const string param" without knowing the real meaning, the resulting code should be considder buggy. Even if it works by accident. Wrongly written code, that works by accident, to me is close enough to being a bug.

Looking at it like this, it is probably correct to say, that this "feature" has cause a lot of bugs already.

But it doesn't mean the feature itself is buggy. It maybe that it isn't documented well enough, as to few people are aware of it. It may even be that the indrotuction of this feature was not a good idea, or should at least have been done in a way that would make the "risks" more obvious. Of course "should have done" is past, little point in discussing that now.

BTW, there are similar pitfalls with functions like fillchar, or move

x: array of string;
fillbyte(x[0], (length(x)-1)*sizeof(string), 0)

If there are any strings with content in the array, it will leak using "move()" on an array like this will lead to crashes later in the code...

Only difference, for those functions more (not all) people know of the dangers...

This is one of the reasons, why I have asked for a range-ceck like feature for such const params (http://bugs.freepascal.org/view.php?id=19605)
- Not only would it help finding where the problem occurs
- It would also help creating awareness:
Many people tend to switch on all the check options (range, overflow, io, stack...) during development. So would they do for a check detecting "const param" issues.
The hope is that simple by noting the features, some would look up it's documentations. The others would do so, once the check raises an alert.

There is no bug.

You can always fool the compiler. The compiler trusts you and assumes that what you tell her is true, namely, that the called code will not modify the const parameter (in the case of an ansistring: a pointer). That's it. Based on this she may or may not perform some optimizations.

For what it's worth, you can recreate the alleged problem just as easily in C or with non-reference-counted classes.

For example:

uses sysutils,classes;
Var
   C : TComponent;
Procedure DoSomething(Const AC : TComponent);
begin
   FreeAndNil(C);
   AC.Name:=AC.Name+'_component'; // You can skip this, even.
   Writeln(AC.Name);
end;
begin
   C:=TComponent.Create(nil);
   C.Name:='Something';
   DoSOmething(C);
end.

Will not produce the desired effect. The compiler cannot warn you against this.

I understand that with Ansistrings it's slightly more convoluted than this, but it's the same principle.

Michael.

Maybe the compiler should start issuing hints or warnings for all places where one uses const with ansistring, so that people who don't care about the speed advantage can start removing those const ansistrings from their code base.

Yes, but is is no bug! It is a contract between the compiler and the programmer in which it is expected that the string will not be modified inside a procedure, function or method.
That is also part of the speed optimization: the compiler doesn't have to check because of this contract. This is also the case in many other languages.

I think you misunderstood me. There is a (or there may be) a bug. But *not* in the compiler, and *not* in the implementation of "const string". I explicitly pointet that out on the top of the quotation "I don't think it is a bug." (with reference to the feature itself)

The bug(s) is/are in user code, where the const construct is *incorrectly* used.

An I was merely pointing out, that any usage without knowledge what it does is incorrect usage (even if by accident it works). Therefore one might consider such unaware/unknowing usage as a bug (a bug in the users code)

ppcontroller.pas(40,31) Hint: Parameter "AStartDate" not used

That's also not a bug, but nevertheless the compiler issued a Hint for it.

That's my opinion as well :-)

All the rumor about hard to find errors IMO is due to the users, which simply should avoid "const" when their overall code then may break itself.

I'd second that "const" with object references is a bug, and should be removed from all library code (RTL, FCL, LCL...). But this is only a matter of style, not affecting compilation in any way.

[examples snipped]

People with according bugs in their code should be happy with an added compiler option, that disables "const" parameter optimization. That's a quick workaround for urgent fixes, where otherwise an in-depth analysis of the entire application were required.

DoDi

That is also part of the speed optimization: the compiler doesn't have to check because of this contract.

More important: the compiler can omit try-finally sections, otherwise required for safe refcounting of non-const strings.

DoDi

2011/7/5 :

You can always fool the compiler. The compiler trusts you and assumes that what you tell her is true, namely, that the called code will not modify the const parameter (in the case of an ansistring: a pointer). That's it. Based on this she may or may not perform some optimizations.

In case of ansistrings, inside the procedure you cannot also finalize the instance that the const parameter points to. But you can modify that instance through other variables. For example :

procedure mclass.dot(const s: string);

Here you know that you cannot modify 's'. But you may not know that you cannot also change mclass.ftext variable, which actually 's' points to because apparently someone called
mclass.dot(mclass.fsometext).

Maybe it is programmer error and it should be said: "never call mclass.dot with mclass.ftext as a parameter because it may crash". Maybe the const should have never been there in the first place. I would say if procedure writes to some non-local strings, never add 'const' to parameter.

It is easy to start adding 'const' to every parameter but some may not be aware of the dangers it may later bring. It should be a well thought decision.

It affects more types, even shortstring suffers from it, the program does not crash but might behave not as expected (similiar examples can be created for normal arrays or records as well):

var
  s : shortstring;
procedure p(const s1 : shortstring);
  begin
    s:='Ops';
    writeln(s1);
  end;
begin
  s:='Hello world';
  p(s);
end.

I see it only as a either or all or none: either the const optimization is done for no types or for all types where it's useful. Everything else is even more dangerous (if you consider this behaviour as dangerous) because the outcome is unpredictible. However, I doubt that people will be happy, if huge data arrays, records or shortstrings are always copied and no const optimization is carried out.

As written in some of the bug reports, I consider as a better solution:
- Pointer checking of constant parameters of types like ansistrings, interfaces at procedure exit. Together with heaptrc and keep_released enabled, this catches all cases of disposed const parameters and probably a lot more pointer errors.
- Like the scrambling of local parameters with -gttt, some checksumming for contant parameters could be implemented. This catches not only the modified constant parameter but also aliasing problems.

There would be the possibility to extend heaptrc mem manager. Every mem block gets a "release-lock"- or const-counter (and maybe a checksum field).

then any memory used in a const param, can be protected (via the mem manager) against release (ONLY if compiled with sanity check).

That would catch the releases due to refcount going to 0, were it must not be allowed.

The checksum can be used for general checking.

The checksum should be toggle-able by a 2nd switch (and maybe with a configurable upper limit for mem size, so to skip tests for very large data)

Variables on the stack, do not get the mem-alloc protection => but they can not be released early anyway

Well, then the following changes are in order:
1) The documentation should recommend users to never use "const string" parameters
except cases where they can prove it to be safe.
Moreover, since the safety of such code may be compromised by completely unrelated changes
(e.g. by adding {$INLINE OFF} for debugging purposes), "const" modifier should not be used at all except the most performance-critical code.
This is quite the opposite of the current practice, where all string parameters are usually
marked by "const".
2) All such incorrect usage should be removed from LCL and FCL (for example: event handlers in LCL and TStrings methods in FCL).

Personally, I think this solution is inferior to my proposal of implementing the optimization
correctly by default.

An I was merely pointing out, that any usage without knowledge what it does is incorrect usage (even if by accident it works). Therefore one might consider such unaware/unknowing usage as a bug (a bug in the users code)

Ok, FPC and Lazarus teams now have many bugs to fix ;-) I''ll start reviewing my TAChart code in a few days.

This seems complicated to me. How about a simple memory poisoning? For example, with the attached patch the code trying to access const string parameter which has been already destroyed is likely to fail spectacularly instead of "maybe working" depending on memory layout.

Alexander Klenin wrote

2) All such incorrect usage should be removed from LCL and FCL (for example: event handlers in LCL and TStrings methods in FCL).

Changing TStrings like that would break the compilation of any code that overrides its methods (since they also have to repeat/leave out const).

Personally, I think this solution is inferior to my proposal of implementing the optimization correctly by default.

Your suggestions about the "safe" cases were wrong, as far as I can tell. As soon as a field, global variable or variable from a parent function (in case of nested functions) is changed, the parameter may also change if it is declared as const (because the field/(global) variable may contain the value that was passed in as "const" parameter). Any pointer may also alias them.

Since the behaviour of "const" for automated types is explicitly defined by Borland as not causing any changes in reference counting (see the note at the bottom of http://docwiki.embarcadero.com/RADStudio/en/Using_Reference_Counting ), I think Martin/Florian's proposal to add the ability for adding extra compiler checks is best. It's similar to how range and overflow checking are optional.

Jonas

I agree with this proposal.

Michael.

You *can* change mclass.ftext, but this can cause *trouble*.

Good advice :-)

DoDi

Hello FPC,

Tuesday, July 5, 2011, 6:44:24 PM, you wrote:

JM> Since the behaviour of "const" for automated types is JM> explicitly defined by Borland as not causing any changes in JM> reference counting (see the note at the bottom of JM> http://docwiki.embarcadero.com/RADStudio/en/Using_Reference_Counting JM> ), I think Martin/Florian's proposal to add the ability for adding JM> extra compiler checks is best. It's similar to how range and JM> overflow checking are optional.

But adding a checksum over allocated data or only over control data ?

This changes should be detected ?

var
  a: ansistring;
procedure DoSomehting(const v: ansistring);
begin
  a[1]:='a';
end;
begin
  a:='b';
  DoSomething(a);
end.

Maybe "simply" add a constRefCount field which is only used with the check automagic types turned on ? Anyway the worst behaviour is refcount related do not ? If refcount should be updated and the variable have a constRefCount>0 then something went wrong. Also this mode should not impose a serious performance penalty.

Alexander Klenin wrote

Lazarus teams now have many bugs to fix;-)

for the LCL we know. Though it isn't sure yet if it is so many.

Finding one of them in the LCL, was actually what started the discussion ....

It would probably best to detect this changes too. But since it would be more of a performance penality, i would like to see it as an option of it's own. (even if only used in debug/development, the penalty could be severe enough to hinter development)

Why detect those?
Well at current such a trick could actually be used intentionally. and it could work well, if done correctly.

But I expect this behaviour is not defined/documented. If the optimizer improves, the optimizer could do thinks that would make any code fail, if it uses such tricks. So better to let people know now, that it isn't safe.

Jonas Maebe wrote

2) All such incorrect usage should be removed from LCL and FCL (for example: event handlers in LCL and TStrings methods in FCL).

Well, then the situation is hopeless ;-)
Note that the only reason TString/TStringList does not break too often is that the only way to get access to an element is by calling Get, which increments refcount
on Result, thus bypassing the bug.
If either one day "const Result" will be implemented (as suggested here: http://delphitools.info/2010/07/28/all-hail-the-const-parameters/) or somebody will, with good intentions, implement something like TStrings.ForEach method,
the number of breakage modes will substantially increase.

Your suggestions about the "safe" cases were wrong, as far as I can tell. As soon as a field, global variable or variable from a parent function (in case of nested functions) is changed, the parameter may also change if it is declared as const (because the field/(global) variable may contain the value that was passed in as "const" parameter). Any pointer may also alias them.

Ah, of course modifying any non-local string variable should turn optimization off too,
I just thought it goes without saying.

Since the behaviour of "const" for automated types is explicitly defined by Borland as not causing any changes in reference counting (see the note at the bottom of http://docwiki.embarcadero.com/RADStudio/en/Using_Reference_Counting ), I think Martin/Florian's proposal to add the ability for adding extra compiler checks is best. It's similar to how range and overflow checking are optional.

I think the link you provided refers to interfaces, not strings. This is the best I could find on strings, which is much more vague: http://docwiki.embarcadero.com/RADStudio/en/Parameters_(Delphi)#Constant_Parameters

Alexander Klenin wrote

On Wed, Jul 6, 2011 at 03:44, Jonas Maebe wrote: > 2) All such incorrect usage should be removed from LCL and FCL > (for example: event handlers in LCL and TStrings methods in FCL). Well, then the situation is hopeless ;-)

TStringList could be made save, and keep the const. Just add additonal references where needed.

At least with respect to the callback evets (OnChanging, OnChange...) inherited classes have to take care for them self.

e.g look at

Function TStringList.Add(const S: string): Integer;
  no events, calls  =>  InsertItem (Result,S);

Procedure TStringList.InsertItem(Index: Integer; const S: string); => event: Changing
insert the new value
=> event: Change

in the 2 event the string could be lost/ For the 2nd event that is no problem (because it has already been added, and got it's reference via the list)

but if it was lost in the first, then corrupted data was added. Yet all that is needed is keep a local copy of the sting to be added, before calling the event

Since the const is kept, "Add " still saves exception frame. InsertItem would need an exception frame ( it may be possible to avoid it, but with a lot of trickery only. not sure)

Note that the only reason TString/TStringList does not break too often is that the only way to get access to an element is by calling Get, which increments refcount

Not getting this part?

The local variable in Add/InsertItem may not have a ref count. but once Added, the entry in the list does of ourse have a ref count...

This does increase the refcount
   Flist^[Index].FString:=S;

Martin wrote

TStringList could be made save, and keep the const. Just add additonal references where needed.

Agreed with your analysis. This still feels like a wrong design to me -- spreading complexity
all over the place (all the libaries) instead of concenctrating it in a single point (compiler).
Still, it seems this is yet another loosing FPC battle for me -- so your suggestion
seems the only resort.

Suppose somebody implements "const Result" as I suggested in the link I posted. Then the code like

  strLst.DelimitedText := strLst[0];

will cause the same problem.

Alternatively, suppose somebody adds TStringList.ForEach(AVisit: TVisitString) method,
then similar problems will be in the visitor procedure

procedure VisitString(const AStr: String);

It's the wrong usage. it shouldn't be used everywhere, then it wouldn't be spread everywhere. the problem was to many people not knowing.

It should be used with the same care and low frequency has move or fillbyte is used. like "move(string1, string2, sizeof(pointer))", very dangerous, but in a few places it may have it's place (TSringlist moves the list of strings (the actual pointers, not the char-data)

Not necessarily

strLst.DelimitedText could hold the new value in a temporary/local var (which would add a refcount) to ensure it does not go away.

And before strLst.DelimitedText overrides the old strLst[0], it has a refcount from strLst[0] => conclusion, if correctly done, it would still be safe.

But again. using const, is like using move or fillbyte on string pointers. The implementer has to know that

Why? As long as the value is in the list, it is referred to by the list?

I have some observations on the discussion so far. The biggest question is what the intended behavior is.

Thaddy wrote

It is a contract between the compiler and the programmer in which it is expected that the string will not be modified inside a procedure, function or method.

This is the crux of the controversy. I realized this when I was writing the original post, but did not mention it explicitly because I thought it would come up anyway.

The difference between a feature and a bug is the specifications. Here the specifications are the documentation. I have not found any documentation in either FPC or Delphi that there is some implicit contract whereby the programmer promises not to modify other variables which happen to refer to the same instance as a const parameter. Many people have repeatedly stated that this is the programmer's fault. If there is an implicit agreement with the programmer, then yes I agree with these statements and I believe it is not a compiler bug (although certainly not good language design).

However I am looking for documentation. Has anyone found anything yet? If anyone can find anything I would be pleased as it would settle the question. But lacking any documentation, I don't see how anyone should know there is an implicit contract. To me, a const parameter means that you cannot modify that parameter by pointing it to something else, nor (in the case of strings and dynamic arrays) alter the contents by means of said parameter. (Although, you can't really "alter" the contents of the instance, as copy-on-write simply creates a whole new instance.) That's what it means in other languages I've used, and nothing more. I don't see it as implying anything else. Furthermore, as many examples have shown, the programmer often /cannot/ know whether several variables refer to the same instance, since the handling of creating and destroying instances, copy-on-write, etc., is handled by the compiler and is considered an implementation detail that should be opaque to the programmer.

I don't know how many of you have actually looked at the demo I posted, but here is the crucial part. The demo program contains this line:

FCurrentDriverName := Edit1.Text;

In this state, the program works perfectly. However, if this line is changed as follows:

FCurrentDriverName := Edit1.Text + 'abc';

the program then crashes. IMHO, this is very scary. All you have to do is make a tiny, harmless change and suddenly a working program crashes. Also in the demo you will notice that the programmer doesn't even call a function that takes a const parameter; the problem is caused by setting a parameter, and it just so happens that behind the scenes the parameter's setter takes a const parameter.

Unless there is some documentation I am unaware of, I don't agree with the implicit contract theory. Instances and variables are not the same. People confuse them because that is actually exactly the point of the Pascal construct: the compiler creates the illusion that each variable is an instance. This is why there is copy on write; so you can modify a variable and it doesn't modify other variables that (prior to modification) referred to the same instance. But again it is only an illusion. In the implementation, a variable (or parameter) and an instance of an automatic type are not the same, and that is where the problem is rooted. The management of these is an opaque implementation detail. The programmer cannot be expected to know whether or not the compiler has chosen to use the same instance for two variables/parameters, and yet that is what the implicit contract theory states.

As in C, Java, etc., if I have a const variable, that means it's a const variable/parameter; i.e., I can't change it to point to something else. It doesn't carry any implications about other variables that may be pointing to the same instance. If the "implicit const contract" is indeed true, then I agree there is not a compiler bug, just a poorly conceived language feature (please note I most certainly am not trying to blame anyone for it though).

The best I have found so far, which is still somewhat ambiguous, is

http://docwiki.embarcadero.com/RADStudio/en/Parameters_%28Delphi%29#Constant_Parameters

which says, "A constant (const) parameter is like a local constant or read-only variable. Constant parameters are similar to value parameters, except that you can't assign a value to a constant parameter within the body of a procedure or function, nor can you pass one as a var parameter to another routine."

Although I acknowledge it is a bit vague, this seems to me to be leaning heavily in the direction that there is no implicit contract. As it says, just think about how a local const variable would behave. Additionally, note that it says you can't assign a value /within the body/ of the routine. There is no constraint mentioned about modifying other variables in other routines. However I acknowledge that the absence of a statement is not proof.

So, the best evidence I have against the implicit contract theory is:
1. By analogy with all the other languages I'm familiar with (C, C++, Java)
2. The fact that if the theory were true it would be a terrible language idea
3. The fact that the Embarcadero documentation likens a const parameter to a local const or const variable.
4. Since using a single instance for multiple variables, copy-on-write, and reference counting are supposed to be opaque implementation details that provide the illusion that every variable is its own unique instance, it does not make any sense to believe the programmer can know when the compiler has chosen to share an instance with other variables, such that the programmer can determine whether or not const is safe in a particular situation.

Again, if there is prior documentation to the contrary, I will gladly be proven wrong, and we can move on to worry about practical steps to mitigate the possibility of problems rather than whether it is a bug. However if there is no such specification, then really what we have here is an undefined aspect of the language. In that case, seeing that it needs clarification, I would propose that a sane language design decision be made by rejecting the implicit contract concept.

Michael wrote

You can always fool the compiler. The compiler trusts you and assumes that what you tell her is true...

Yes, of course you can always fool the compiler, it just shouldn't be the other way around. The example you gave is very different for one very important reason: you show using explicit allocating and freeing of an object. With strings, the programmer does not, and cannot, explicitly allocate or deallocate the resources, and the problem lies in the behavior of the automatic allocation and deallocation. Thus there is

Wimpie wrote

If it helps, I can remember fixing this bug. It took two days...

...

So yes, it is very scary to me and would like it to be fixed.

Thank you for providing some corroborating evidence that there is in fact real-world danger here and that it is worth the time to consider.

Florian wrote

It affects more types, even shortstring suffers from it

I must respectfully disagree. In the case of shortstring, the value of the const parameter does get modified, but that is to be expected. If my understanding is correct (and I'm open to be corrected), the semantics of ShortString are different. With AnsiString, assigning one string variable to another is supposed to create the illusion that they are unique instances. Hence there is copy-on-write. With short strings, assigning one to another literally means they are the same instance. Again this comes back to the difference between instance and variable, and the illusion implicit in AnsiString and dynamic arrays, which I think is not the case with ShortString (but again I could be wrong).

The problem at issue here is the fact that the compiler can actually free memory prematurely. In the case of shortstring, it won't crash. The programmer may have tricked himself, but the compiler isn't doing anything unexpected. In the case of reference counted types (strings, dynamic arrays, and interfaces), the compiler is doing something quite unpredictable. Again, however, this depends on the specification/documentation. If there is in fact a constraint the programmer must follow of not modifying other variables that by chance point to the same instance, then I would agree there is no problem with ref counted types either and that the programmer has made an error. I just question that assumption.

If the compiler is actually working correctly and there is an unstated (perhaps even undocumented) constraint on the programmer to not modify other variables that use the same instance as a const parameter (even though the programmer cannot know that), then I fully agree with this advice.

In fact, with the implicit contract theory, I propose that it would *never* safe to use const, because the programmer cannot control the implementation of the ref counted types. It is an explicit part of the design that the programmer should not, and indeed cannot, be aware of when/where the compiler allocates, deallocates, or copies instances of any particular ref counted type. Therefore, semantically there can never be a guarantee that using const is safe, if the implicit contract is accepted. I stress, instances and variables are not the same. The relationship between them is supposed to be an opaque implementation detail. That is part of the reason I do not believe in this implicit contract about handling consts.

Now I will acknowledge that const can be used in certain limited situations without harm. I still stand by the claim that it is semantically incorrect to ever use const, and I say this on the basis that if const safety depends on the automatic type implementation and the automatic type implementation is opaque, it is theoretically wrong to believe const can ever be used. However in practical terms we do know that passing a string which is *created* as a local variable in a routine cannot do harm because it will always have at least 1 refcount.

To summarize:

1. To the programmer, each AnsiString and dynamic array variable is supposed to be unique, i.e., after doing A := B, modifying A does not affect B and vice versa.

2. The fact that multiple variables (or parameters) actually can refer to a single instance is an implementation optimization. That is of no concern to the programmer. The optimization is implemented by the combination of reference counting and copy-on-write.

3. The programmer cannot be aware of what the compiler decides to do regarding how it implements reference counting and copy-on-write. The programmer should simply know that unique variables are unique instances for all practical purposes (except var parameters obviously, since that is the whole point of having to declare it var).

4. If the programmer cannot be aware of when an instance is shared with multiple variables, an implicit contract that the programmer cannot modify other variables which by chance are using the same instance is impossible to obey, and therefore const would be useless.

As further support for #1, consider: With a class, you do not have to pass it to a procedure as var, but when you modify the instance, you modify the same instance as was passed to the procedure. That is how classes work. Having a variable point to the same object as another variable does in fact mean it's the same instance, the same object. That is Object Pascal's object model. But with strings, if you want to modify the string in a procedure and have that affect the argument initially passed to the procedure you *have* to use var. That alone should be convincing evidence that the programmer is always supposed to be able to assume that unique variables are unique instances for automatically managed strings and arrays.

If I may offer one other comment, I think it would be best to reserve comments on TStrings/TStringList and other libraries to a separate conversation and focus on the basic behavior of the compiler or else the conversation will get too confusing.

Chad, this line is worthy of place among the top programmer's quotes ;-) I think it is an excellent summary of your (and my) position.

I, like you, think it is a bug. I'd even go as far as to say that, if it is specified somewhere, then *the specification is buggy too*. However, given the response from FPC developers, and based an my prior experience, I think it will be left unfixed. Hence my alternative plan above.

In fact, with the implicit contract theory, I propose that it would *never* safe to use const, because the programmer cannot control the implementation of the ref counted types.

Yes, I meant the same when I wrote "except the most performance-critical code", this is to become dangerous, rarely-used and non-portable optimization technique,
like using asm code.

Aside question -- are FPC iniling decisions platform-dependent?

1. To the programmer, each AnsiString and dynamic array variable is supposed to be unique, i.e., after doing A := B, modifying A does not affect B and vice versa.

AFAIK, arrays are not refcounted.

This is also a good argument which I omitted before in the interests of brevity.

Put in another words, current "const string" implementation honors the implicit contract of
"adding const in front of the string parameter cancels refcounting", at the cost of breaking the implicit contract of "refcounted values are indistinguishable
from simple values (except maybe in performance)". If some specification includes both contracts, it is contradictory -- and thus, in my opinion, buggy.
So one of the contracts should be removed -- and obviously FPC developers think that the first one is more important, which I disagree with.

If I may offer one other comment, I think it would be best to reserve comments on TStrings/TStringList and other libraries to a separate conversation and focus on the basic behavior of the compiler or else the conversation will get too confusing.

I think the discussion of the compiler behaviour is already over, and we lost. So there is no harm in discussing fall-back strategies ;-)

As I have shown before in this thread, non ref. counted values e.g. shortstrings are affected by the same problem.

And as per Chad's reply (and agree with his logic), this is not so.

where there is no 'allocation' or 'Deallocation'. Such examples can wreak havoc just as much. With the example I showed, it's just much more clear. (i.e. a straight crash)

Michael.

It is a general problem of languages supporting pointers that you can change "const" data. Const can never protect 100%. Maybe this should be mentioned more prominent in the docs, probably with some examples, although I don't know where a good place is.

And it is a general problem that compile time checks/protection only works for a limited scope. Code outside the scope can not be checked by the compiler. A const parameter is only checked for the scope of the body. If you leave the body then the compiler can not help. And sharing memory (s1:=s2) is a run time problem, which means it is outside of the compiler scope too.

SetLength, UniqueString, ...

Yes, but it is documented that it is only an illusion. AnsiStrings share data.

A crash is just a boundary check by the OS.
With Ansistring the crash happens some time after the real bug. And it does not need to manifest as a crash. The same happens if you do that with shortstrings or records or whatever.
As Florian already wrote this belongs to a whole category of nasty bugs and there are tools to find them.

[...] Now I will acknowledge that const can be used in certain limited situations without harm.

The term "limited" is pretty wrong. For example const string is used at several thousands places in the lazarus and fpc sources. And afaik they were used wrong less than ten times. In many cases it was right and became wrong, because some called function changed.

That's only true for a limited set of functions and operators. For example using Move on the characters will modify both.

2. The fact that multiple variables (or parameters) actually can refer to a single instance is an implementation optimization. That is of no concern to the programmer.

Huh?
It is one of the main reasons to use ansistring instead of shortstring. And many implementations rely on the runtime difference O(1) vs O(n).

The 'all practical purposes' is not clear.
I guess some people think a Move is a practical function.

4. If the programmer cannot be aware of when an instance is shared with multiple variables, an implicit contract that the programmer cannot modify other variables which by chance are using the same instance is impossible to obey, and therefore const would be useless.

And still it is not useless.

[...]

Mattias

Well it's not as obvious)

In Florian's example the "const" param follows the value of the original variable (so yes it does something that is unexpected) But with short string it does not crash. there is no ref count going wrong, there is no memory being freed while still in use.

Yet he is 100% right it's the same problem, the same error, and as severe.

Because the behaviour is not guaranteed.
The compiler (in future versions) could make optimizations knowing that the "const s: shortstring" (or "const x: sometype") doesn't change.

And that means, in future such code could behave different or even crash, only by change of content (even if memory is not freed). Hence yes, it is as severe as with ansistrings. It only is not yet as visible.

Here is the example as provided by Florian:

var
   s : shortstring;
procedure p(const s1 : shortstring);
   begin
     s:='Ops';
     writeln(s1);
   end;
begin
   s:='Hello world';
   p(s);
end.

*Aliasing* is a common problem, not restricted to const parameters.

A subroutine may e.g. free a global object, and replace it by its argument. Calling this subroutine with that global object results in an invalid reference in the global variable.

In C "const" has several meanings, comparison with other languages is not a good idea.

In OPL "const" means that the argument will not be altered inside the subroutine, so that the compiler can apply some optimizations[1]. That's all. The compiler *can* check for e.g. illegal writes to an const parameter, but there is no such *obligation*, and it's known that the compiler can be fooled by trickery, in any language.

[1] E.g. const record parameters can be passed by reference, instead of passing a copy (by value) to the subroutine. Managed types can be passed without refcounting and associated try-finally constructs. But there exists no guarantee, which compiler will apply what optimization, under which conditions.

I do not defend critical optimizations in an compiler. E.g. it's known that managed types with a RefCount of zero should not be passed as const parameters, *when* inside the subroutine the RefCount can be increased and decreased again. Here the compiler may have a chance to report an *possibly* inappropriate use of "const".

DoDi

Martin hat am 7. Juli 2011 um 12:20 geschrieben:

You don't need the future. You can already create a crash using const
shortstring:
 
 
{$mode objfpc}{$H+}
 uses Classes;
 var
   s: shortstring = '';
   o: TStringList = nil;
 procedure DoSomething(const h: shortstring);
 begin
   if h'' then
     o:=TStringList.Create; // not called, because s=h=''
   s:='A';
   if h'' then
     writeln(o.Count); // called, because now s=h='A' => crash
 end;
 begin
   DoSomething(s);
 end.
 
 

[...]

Mattias

Mattias Gaertner wrote

You don't need the future. You can already create a crash using const shortstring:

Indeed, thanks for the example.

And with this example there is another potential issue arising from not honouring the contract of const.

The behaviour of such code may change at random depending on fpc-version or optimization. Future versions of fpc could cahce the result of "h''" (as this expression is constant too), and re-use it in the 2nd IF. So mis-honouring the rules of how to use "const" does not only lead to potential crashes, but it also leads to code that has no defined behaviour, and may lead to completely random results.

This means the current behaviour with ansistring is absolutely right. It is only a consequence of the behaviour already observed with other types.

Sigh....

That's what I've been saying all along. You don't even need strings for it. Plain records and integers will do it just as well.

Michael.

Sigh.... That's what I've been saying all along. You don't even need strings for it. Plain records and integers will do it just as well.

Well, I am sorry, but do you all (not only Michael, I'm just responding to the last mail in the thread)
really think I am stupid enough to not know it?
It is quite understandable that FPC developers want to defend the status quo, but can you please do it by responding to the argument OP and I actually make?

I'll re-iterate again:
1) The reference counting is not just an optimization, it is also a memory management technique,
an important concept which is aimed at providing "simple value" semantics for heap-allocated structures,
and insulating the programmer from memory allocation details. If programmer does not do anything obviously dangerous like direct memory access,
using strings is guaranteed not to cause a whole class of memory errors, like leaks, double-frees and access-after-free.
Current "const string" implementation breaks this guarantee for the sake of minor optimization.
It is actually quite ridiculous: "var s: String" *is now safer* than "const s: String",
because the former can not cause AV, but the latter can.
2) Also, there is a separate-but-related issue that Object Pascal traditionally conflates
"const" and "reference" semantics in parameters. It would be significantly better to have
separate "const" and "constref" attributes, but I guess it is much too late. Nevertheless, currently there are two groups of types in FPC, "simple" types like numerics for which "const" acts like "passing by value", and "complex" types for which "const" acts like "passing by reference". Note that currently "const string" *is passed by value*, *except when the reference count equals to 1*, when it suddenly behaves as if passed by reference.

I do think Florians, or Mattias example are relevant. Let's for a moment ignore the const ansistring issue.

The 2 examples prove, that even with "const i : SomeRecord", your code can crash for none obvious reasons, IF you break the promise, not to modify the value of I, by modifying the value that you passed in.

That is:
"const x: sometype" for any type (with the exception maybe of those , that are passed by value) can lead to undefined behaviour and/or crashes, if the promise of "const" is broken.

If the above statement can be agreed on (and the examples show this), then why should the "const ansi string" be a bigger issue?

Yes, the const ansi string breaks the rules that ought to be provided by ref-counting.
But it only breaks this ref-count promise, for code that is already unsave, even if ref-counting was in place.

As I have stated in my reply to Mattias, future fpc, could do all short of optimizations, caching the result of expressions that use only constants, and const param => the result of such expressions has to be constant too.
So even if ref-counting was in place. The code was unsafe, because it's behaviour is not defined, and it's results could randomly change in future.

Given that the very same code would still be unsafe, even if ref counting was there, then why put ref-counting in there?

actually constref exists.
but what you seem to want, which is missing: ConstButNotRef

The problem is your understanding of const.

The declaration

Procedure DoSomething(const A : Type);

means:

YOU promise not to change A inside DoSomething.

Based on this promise, the compiler may decide to do some optimization. It will also warn you if it detects inside the code that you break your promise.

The declaration

Procedure DoSomething(const A : Type);

does NOT mean:

The compiler promises that A does not change inside DoSomething.

You seem to think that this should be the case. Well, it is not the case.

Const is an aid from you to the compiler. Not an aid from the compiler to you.

As soon as you understand this subtle difference, the problem disappears.

Michael.

Based on this promise, the compiler may decide to do some optimization. It will also warn you if it detects inside the code that you break your promise.

Based on my new knowledge of const strings I revisited the bug I mentioned previously. I still have no idea where exactly I made the error. And I never got a warning about the promise being broken.

For me the advantages of using const strings are completely dwarfed by the time required to ensure I keep my side of the promise and still have the risk that some small change in the future will break it without causing compiler warnings.

As soon as you understand this subtle difference, the problem disappears.

My suggestion is that the gist of the documentation be changed from "Use const strings, it is good for you" to "Const strings are powerful. They are also dangerous. Don't use it unless you really really understand them"

Well, I program FPC and Delphi since many many years, in a multithreaded server environment, and have never encountered a problem with this.

Given this, and the IMHO convoluted examples that were used to show the 'problem', I'm inclined to say that you must be writing some pretty strange code to have encountered this problem.

Given that Borland never decided to 'fix' it, I'm inclined to think that they also don't consider it a real problem, but rather a corner case (if they are aware of it at all).

That of course doesn't alter the fact that a corner case can seriously bite you in the leg. I've had such cases as well. I just shrugged and fixed my code.

C'est la vie.

Michael.

The original example ( a bug on mantis) had little strangeness. A global variable being modified in an OnChange handler of a TEdit. This could very, very easily happen.

I think it is correct to say that there currently is a problem, and it starts in the documentation, and results in (many/most) people not knowing. The extend of this "not knowing" (or at least not fully taking in account all consequences) is manifested in both the LCL and the FCL.

Both of them include code, that invites to break the promise. That is, it does not in itself break the promise. But I hides the promise made from the user, and therefore the user can break the promise without ever knowing it was made.
This happens when code provides events (such as many LCL Controls, or TStringList)
They accept strings (and there types) as const param to some methods, and then call back user code, in an event. This user code can modify the originally passed in variable.

Sure it could be said, that if I pass SomeValue to TStringList.Add, then I should see that I pass it as const param, and should know that I must not change it.
Only:
- These classes (TStringList, TControl) are provided to all users, including beginners. They contain no further warning.
- The user may not see the "const" declaration

And the user can not be sure, that he must follow the promise. TStringList could internally be protecting the value, and the user would not need to care about the promise.

In other words, the fact that such an expert option has been exposed in such manner to even beginners, implies that the authors themself had not fully reflected all consequences.
( At least in the LCL this has happened due to such unawareness)

Have I not just written, in the very detailed manner, that reference counting is a technique specifically designed to prevent this problem? That you code CAN NOT crash, much less for non-obvious reasons, if you use ansistring?
That this is a critical advantage of using it, as compared to, say, shortstring? That, finally, the examples with pointers and manual memory management are proving just the opposite of what their author wanted -- i.e. current implementation makes AnsiString no better than PChar in terms of safety?

If you look at the point (2) of the mail you responded to, you will see that I discuss this exact issue. "Const string" is passed by value, and it does not lead to any undefined behaviour and crashes except this case (barring, of course, as-yet-unknown compiler bugs).

Yes, the const ansi string breaks the rules that ought to be provided by ref-counting. But it only breaks this ref-count promise, for code that is already unsave, even if ref-counting was in place.

How so? If the bug would be fixed, any procedure with "const string" parameter will become perfectly safe, just as with, e.g., "const integer".

Given that the very same code would still be unsafe, even if ref counting was there, then why put ref-counting in there?

I am somewhat unsure what are you talking about here, but if you mean the "const string" parameters, then the statement above is incorrect.

actually constref exists. but what you seem to want, which is missing: ConstButNotRef

Yes, but with less ugly syntax ;-)
Maybe "constval"?

hat am 7. Juli 2011 um 13:10 geschrieben:

[..] That's what I've been saying all along. You don't even need strings for it. Plain records and integers will do it just as well.

Yes.
I hope the example helps people understanding this.

Maybe an example can be added to the documentation to explain that 'const' is not a total protection.

Mattias

I had already planned it.

Michael.

Given that Borland never decided to 'fix' it, I'm inclined to think that they also don't consider it a real problem, but rather a corner case (if they are aware of it at all).

Hm. My testing indicates that Delphi has this fixed since at least D2007.

(code below written in my mailer, so my have typos, but should be enough to get the point

program Foo;
var Bar: ansistring;
procedure Test(const s: ansistring);
var x: TStringList;
begin
   if s[1] = '<' then x := TStringList.Create;
   Bar := '<<<';
   if s[1] = '<' then x.Add('a'); // crash, because you changed Bar
end;
begin
   Bar := copy('afhdsgfyowahfol',2,4);
   Test(Bar);
end;

However the Example only demonstrates half the problem. The above *HACK* could be mad working. Within todays FPC it could even intentionally be used....

EXCEPT:
The above hack is unsafe. It exploits undocumented behaviour. With any future FPC it's behaviour could totally change.
Any code that relies on an undocumented randomly found out detail about how the compiler *currentlly* translates some code, is very bad code: destined to crash some day.

Because if "s" is constant, then so is if "s[1] = '<'"=> so once this expression has been calculated, the compiler is free to keep the result cached. And that would change the flow of the above code.

Of course in the above example that would actually stop it from crashing, but in other code, it may start it crashing, or just return different results from what they used to be.

-------
So in conclusion. Even if the above code did have ref counting, it would not work (code that changes it's behaviour at random due to compiler optimizations is not working).

Oh yes of course refcounting would do copy on write? well cast it to a pchar, to avoid the copy on write. It's an example, it's consrtructed to show the possibility of the issue in the minimum space, Hence it is not the most "common world" code. But it means it can happen in common world code.

maybe it was a field on an object. Same problem

The original example ( a bug on mantis) had little strangeness. A global variable being modified in an OnChange handler of a TEdit. This could very, very easily happen.

One of the reasons why *global variables* should be avoided.

DoDi

[example skipped]

Above, I have quoted the relevant part of the mail you responded to. If you read it, you will note that it discusses exactly the case you tried to demonstrate.
Your example (after fixes identified below) proves my points, which are, yet again:

1) That the code you posted would work quite correctly if "const string" is fixed,
including all the optimizations you are suggesting in the rest of your message.
2) That it works correctly in (recent versions of) Delphi
3) That is will break in unpredictable fashion due to the bug we are talking about.

Note: besides minor typos, you example does not cause error due to unrelated issue: for some reason, string value returned from function has a refcount of 2 instead of 1.
I replaced call to "Copy" by "'x' + Bar + 'x'" to ensure refcount of 1.

No, it did not.

program tests;
{$APPTYPE CONSOLE}
uses
   SysUtils;
Var
   A : ansistring;
Procedure DoIt(Const B : ansistring);
begin
   A:='Something else';
   Writeln(B);
end;
begin
   A:='Something';
   DoIt(A);
   Readln;
end.

Writes 'Something', because the pointer B points to a block that has not yet been invalidated. It works by accident.

Changing it to:

program tests;
{$APPTYPE CONSOLE}
uses
   SysUtils;
Var
   A : ansistring;
Procedure DoIt(Const B : ansistring);
begin
   A:='Something else';
   Writeln(B+' aha !!');
end;
begin
   A:='Something';
   DoIt(A);
   Readln;
end.

The program crashes.

If you look at the generated assembler code, you'll see that no reference count increasing is done in DoIt.

Michael.

You are right, sorry. I have managed to bork my testing by inadverently incrementing refcount.

No.

As I said: IF the result of constant expressions like "s[a] = '<'" are cached (in future) depending on optimization level, then it breaks.

The 2nd evaluation, of this expression without optimization returns a different result from the 1st evaluation. (It may be that the example would need updating, but there is code where this can happen, including if ref-counting was done)

So depending on the 2nd evaluation being done, or cached (future optimization) the program flow differs.

Code which behaves unpredictable is broken.
In this case the none predictability comes from the fact that the code relies on a randomly observed and undocumented behaviour of todays fpc implementation. (the fact that today no such optimization is done)

Well, maybe you should run the code and see for youself -- I do not know how to explain this in even more detail. It will *only* break due to the current optimization bug. Remove the bug, and no breakage is possible,
with or without caching you describe.
Even now, if you ensure refcount>1, no breakage is possible.

It only differs now due to the current optimization bug. Remove the bug, and the code will stop breaking.

Code which behaves unpredictable is broken.

Exactly. This is the problem I hope to fix.

Ok, so here I go, an example without "const" => that means the refcount is increased.

Yet it crashes.
But the crash is NOT the issue. After all this crash is not caused by "const". there is no const.
this crash is simply wrong code....

But it shows something else.
It shows that even with refcount in place, it is possible to modify (via a global var) the content of a local string.

Yes, I use pointers, but it does not matter how I managed to change the content of "s". All that matters is, that I broke the promise (assuming s was declared const")

But as soon as the content of s changes (and if s was declared const), the behaviour is simply unpredictable.

Because the second "s[2] = 'x'"  *could* be cached *or* evaluated;
leading to a different execution path

-------

program Project1; {$mode objfpc}{$H+}
uses Classes;
var Foo: String;
procedure Ouch(a: pchar);
begin
   a[0] := 'x';
end;
procedure Test1(  (*const*)  s: String);
var x: TStringList;
begin
   if s[2] = 'x' then
     x := TStringList.Create;
   Ouch(@Foo[2]);
   if s[2] = 'x' then
     x.Add('lets hope');
end;
begin
   Foo := 'ab'+'12';
   Test1(Foo);
   readln;
end.

A different point about this.

What I understand you want is, for "const s: string" to do increase the ref-count. (regardless of any other issues there may or may not be).

But from today's point of view, from what "cons string" does today: What is the point in doing this.?
Today this would be equal to simply remove the entire feature.

AFAIK, today all that cons string does is remove the ref count.

So basically you say, because of your reasoning, no one should be able to use such a feature in there code?

Martin wrote

Yes, I use pointers, but it does not matter how I managed to change the content of "s". All that matters is, that I broke the promise (assuming s was declared const")

No, the whole point it is that it *does* matter.
Direct memory access lets you break anything --
class members' visibility, callstack frames,
built-in data structures like vtables, exceptions, refcounting, does not mean all those features are useless and we should go back to programming in assembler.
That also does not mean we should allow arbitrary breakage of those features since "you can break them anyway".

After all, you can break any string code the code much simpler:
(PInteger(s)-1)^:=-1;

It should do what is semantically natural for "const" -- namely, prevent programmer from modifying the variable so declared. Cf. "const Integer".
I understand that "const" meaning in Object Pascal has long since lost it's meaning, but still, what it should *not* do is breaking user programs unexpectedly.
Just step away and think about the situation:
what would you think if, say, adding C++ "const" modifier would convert working C++ program into subtly broken one? Note that I am not against the optimization itself -- just against the optimization which changes the meaning of the code. Imagine, again, if the optimizer would, say, pull repeated calculation out of the loop without checking if the variables may be changed by some procedure called from the loop body. Also note that, implemented properly, removal of incref/decref calls should not depend on any const declarations, it can be performed even for non-const strings.

Also note that, implemented properly, removal of incref/decref calls should not depend on any const declarations, it can be performed even for non-const strings.

I'am awaiting your patch.

So you agree with my design?

ok, so ansistring are the exception (or at least I will not search for further ways), because their copy-on-write adds an extra layer of protection.

but it still doesn't help.

Florian showed that the problem exist for ShortString.

In many cases the difference between ansi and short string s $H+/-. Which means you move the breaking of your code into a compiler switch.

And also the example with the cached evaluation, would also apply to let's say records.

So in your scenario, the "const param" would then mean:
- const x: record => a promise not to modify the original record, or else you risk crashes and other errors
- const s: string => no promise, so you can do what you want

That doesn't make much sense either. Now const means different things depending on the type ?

About detecting if value parameters need no incr/decr. ref? I didn't see any design yet.

....

Also note that, implemented properly, removal of incref/decref calls should not depend on any const declarations, it can be performed even for non-const strings.

Differnet feature. Nice to have...

But no compiler can identify every occurrence where the ref count can be ommitted.

so it still stands:
What you want, is the removal of a feature from pascal. The feature for the programmer to explicitly skip the ref count (at the programmers own risk)

If that is the case: Should we remove "move" / "fillchar" too? Because they can do the same.

Martin wrote

That doesn't make much sense either. Now const means different things depending on the type ?

It already does. I agree it makes no sense, but this is much larger problem, which deserves at least a separate discussion thread.

Currently, there are four meaninigs of "const":
1) "Const by value" -- like Integer
2) "Const by reference" -- like shortstring
3) "Const by reference, but not really const" -- like objects
4) "Const by value, excapt rare breakage" -- AnsiString (and interfaces, but let's not touch that can of worms in this thread :-) )

I propose to remove meaning (4).

Unlogical. 2) and 4) are coupled.

See http://bugs.freepascal.org/view.php?id=19605 and this thread

This whole construct is unlogical. Logically, "const a" should mean just "assignment to a is a compile-time error".
But this is not achievable due to compatibility reasons. Still, I suggest that (4) is a (small, but still) evil, and sould be removed.

Alexander Klenin wrote

> > About detecting if value parameters need no incr/decr. ref? I didn't see > any design yet. See http://bugs.freepascal.org/view.php?id=19605 and this thread

that is about at runtime, in debug compiled exe

We are talking about compile time decissions

Well, filter out all notes except mine then ;-)

Why? 2) is exactly the same? The const contract with the compiler is broken.

Besides this, people like Martin Schreiber won't be happy if a procedure suddently eats hundreds of extra clock cycles for nothing.

Agreed.

Well, don't you think that "expliciitly ask to skip refcount" feature is quite different from "constant parameter"?
At some stage, semantic annotation got conflated with a specific optimization it allowed.
I propose to decouple them -- let the "const" mean "constant" (which, by the way, can not be achieved currently, so it would be new feature), and do refcount skipping only if safe
(or, for the really critical codepaths, introduce a separate, explicitly-named annotation,
something like "const norefcount").

If that is the case: Should we remove "move" / "fillchar" too? Because they can do the same.

There is Move and there is assignment.
You would not advocate always perform "Move" on assignment, beause it is slightly faster, even if it breaks some code like AnsiString assignment?

Florian Klämpfl wrote

Why? 2) is exactly the same? The const contract with the compiler is broken.

Since when are passing by value and by reference the same?

Besides this, people like Martin Schreiber won't be happy if a procedure suddently eats hundreds of extra clock cycles for nothing.

For those people, I proposed to add optimization switch to the "UNCERTAIN" group. I just do not think it should be on by default. Also, can you Cc him -- maybe we should ask him directly?

Slightly fresh start.

I do believe the feature itself is a valid feature (the ability for the programmer to tell the compiler to skip something like ref counts) There is a similar feature (via compiler switch) to skip implicit exception frames. (With the risk of mem leaks, and what else, if incorrectly used.

The current naming may not be the best, since it implies a point of view different from the real meaning.

But the current naming is already there, shall we really make everyone who used the feature correctly, change their code, because some others did not?
Wouldn't that be to punish the ones who got it right.

I don't oppose having compiler switches, and/or directives, to defined the exact kind and extend of the const behaviour, so people can choose the one they like.
- But I can see, that those who maintain the overall compiler-code will cry out in pain, in expectancy of what there will be to be maintained...
- And I am not sure, if any other more correct definitions can be done (The problem is, I am not sure, if any other design, does not end up in splitting hairs, due to the overlap of tightly coupled issues....)

I agree the fact that objects are not const, if passed as const is not consistent. But if that was to be fixed, then objects had to be made real const too (rather than further weaken the entire thing)

Anyway back to strings.
I would not oppose (but I am not sure if sensible either) if the behaviour for strings could be modified by compiler switch or directive.

To prove or disprove the sense in that, will take a lot of tiny steps, and see , if in the end they all make sense. And I will not go into that discussion....
Just some ideas what all might have to be considered, extremely incomplete list of ideas. Maybe easy to dismiss at first sight, but before dismissal they would have to be tracked down a long way, to every possible extend. Feel free to dismiss them, I have no intend in proving them.

- You mentioned interfaces, well there are dyn-array ref counted too and no copy on write. So the earlier example I tried to make on strings (with the caching of expressions) it would probably apply to dyn arrays... Which means trying to change the feature, would create even more and more special cases... (or alternatively for all time forbid certain optimizations like caching certain results...)

- const should avoid to copy data (only data that is smaller or equal to pointer is passed as value). But strings with refcount will be copied (if the other/global var with the same data is changed). So adding a refcount, means making a copy of the data. Which is fundamentally against the stated indent of not copying it...

....

Not necessarily:

if const reads: I (the programmer) tell the compiler this value is going to be constant
versus: I ask the compiler to help me avoid changing it

But as in my other mail, it may not be best named (but now we are changing topic, it's not about the feature anymore, it's about the name)

At first sight (and first sight only) I would say, if it wasn't yet there and a name was to be choosen, I'd choose another name.

But at 2nd sight, I retuned to the example with the caching of expresion results.. And If the feature would really mean const "the value of this variable will not change, not from within nor from outside this function" (and that is what in case of a record (passed by ref) is needed to be real const, and allow all the optimization. Now if I declare const should actually mean that, I am back to where we are no. Only stricter...

in c++ you can mark an entire method to be const. Means that method can not call any thing, that modifies the object.
If I wanted to go all the way in pascal, then "const x: type" should mean:
- the compiler allows no modification of x in the procedure (exists in fpc)
- the compiler allows x only as param, if the param is const too (exists in fpc)
- (new) the compiler prevents calling any other code (any method, procedure, or event) unless it has proven knowlege that this code does not do anything that could modify the value of x
eg only calls to methods that do not modify any variable of the type of x at all (and that do not call anything that does...)

If it was anew feature...

"advocate"

Neither do I advocate to use const, unless you know what you do..... And if anyone advocates it, then it's a problem (but not a problem of the feature)

Sure.

But the current naming is already there, shall we really make everyone who used the feature correctly, change their code, because some others did not? Wouldn't that be to punish the ones who got it right.

The problem with this particular feature is that *nobody* seems to use it correctly --
if even compiler developers did not get it right, what can we expect from ordinary
users?

About the rest of you ideas -- they certainly have some validity, but I do believe they can be discussed separately.

Well, that difference is a bit on the of philosophic side in general case, and, as I said, totally changing the meaning of "const" is not what I argue for in this thread. (IMHO "const" should mean both). But, in the specific case of "const string" parameters in FPC, the problem is that it *already* means both,
and I suspect a lot of code depends on the current meaning. So this specific case, while maintaining the
contract of "no refcounting on const" runs contrary to the (IMO) much more meaningful contacts like "adding const increases safety of the code" and "no memory errors from using ansistrings without messing with pointers".

I do not quite understand this passage, but did you note that currently the caching you propose would be unsafe for strings, but you can make it safe if you fix this bug?
That is, my proposal increases the possibilities for optimization.

Note the distinction here: compiler must only refuse code it knows to be incorrect,
but must only make optimizations it knows to be correct. So the right answer for two latter cases is to allow them, but disable optimizations -- which is exactly what I proposed in the first place.

Neither do I advocate  to use const, unless you know what you do..... And if anyone advocates it, then it's a problem (but not a problem of the feature)

Just google "const string parameter Delphi" to see the advocating.

That's a different issue.

I have stated several times that to me "const param" is similar like "fillchar(StringArray[0], x, 0)"

for both you need to be aware of certain details abstracted from the average use.

Here I am. Probably the most ordinary FP user on the planet. Just let me throw in this discussion, to be honest, for years, I do not have any issues with const parameters, related to this debate. The "contract" is completely ok for me. And I always understood it as my promise to the compiler, as Michael pointed out. Not at all reversely, as the compiler's commitment.

So I am perfectly happy with this promise. Except, sometimes, when an related error is thrown, I have to remember again, that "const" automatically excludes dynamic function or operation results as a parameter, as well.

From my personal point of view, THIS has nothing to do with the promise,

not to modify a parameter, declared as "const", during the called routine is running. But I cannot imagine that there is any chance for a change, since it's probably always done like this, in each and every version of the compiler.

Reyno.

I have just written up some very compelling arguments and then decided to scrap it.

I maintain that const strings, dynamic arrays, and interfaces, as currently implemented, are dangerous and should be considered useless in all but the most limited circumstances. The behavior is dependent upon implementation details that can change with compiler version, platform, optimizations enabled, etc. Unrelated changes in the code can also trigger bugs. Examples can be given which are extremely simple, and others which are extremely complex, showing how hard it can be to ever really trust the use of const.

The problem comes down to the specs though, or rather the lack thereof. As I have searched the web and read some docs at Embarcadero, things have only become more ambiguous. It seems that everyone repeats the same basic explanations but never fleshes out the subtleties. In my opinion, if you compared C, Java, and C++ (to a slightly lesser extent) to a mathematical proof, then Object Pascal in today's form would be more like a pile of notes and some lunchtime conversations.

Basically, I think the current behavior is bad, but whether it is a bug depends on the specs, which are lacking, and therefore I am no longer certain it is a bug. With what I have read, I no longer feel that a provable argument either way is possible.

I'm more interested now in a solution. The solutions I've seen so far have potential, but in my opinion most of them seem like they are more complex, would be more overhead, and produce worse performance than just getting rid of the const "optimization".

That being said, I can't complain because I don't have anything better to offer. My knowledge of FPC internals is just not enough (it is very, very little, actually).

However, looking at the generated assembly, I can't believe there isn't a more efficient way to handle the implicit try-finally. It feels to me like that is where the problem is. There is a lot of code and several function calls that go into implementing that implicit 'finally' block. There's got to be some way to decrement the refcount even when there are exceptions but without so much overhead. It would require changes to the exception handling mechanism though. Here my knowledge runs out.

For now, I will just agree that I believe the most practical thing to do is: As has already been said, have it be an optional optimization, off by default.

Secondly, I'm considering using var everywhere. It's ugly--very. But the performance of copying strings is not acceptable, and the danger of using const is not acceptable, except in limited well-controlled circumstances--but even then it's dependent on some unstated assumptions about the compiler implementation.

BTW, has anyone looked at my demo sufficiently that they can explain exactly what is happening? I do know what it's doing, but I think you can only really appreciate why this is dangerous if you see how subtle it can be. We can code very carefully, but in a large project, especially over time, with many people modifying it, I don't see how you can possibly believe using const is safe. It can sneak in so easily in unrelated parts of the code. That is probably what is scariest to me, and why I don't consider it safe no matter how careful you are. Personally, I feel that in the current implementation, it would be better for const to not even exist. I am uncertain of what I will do in my own Pascal coding.

Op Thu, 7 Jul 2011, schreef Chad Berchek:

The problem comes down to the specs though, or rather the lack thereof. As I have searched the web and read some docs at Embarcadero, things have only become more ambiguous.

You are looking in the wrong places. Both Turbo Pascal and Delphi came with a language guide that describe the desired behaviour in deep detail.

Daniël

It's true that C, Java and C++ all have some kind of committee that designs and fleshes out the specs, which are then published so that others can conform to them. They go into excruciating detail about minor details to make sure they are as unambiguous as possible. For Pascal this also happened in the past (the ISO standards), but Borland's success was largely based on deviating from those standards and coming up with their own dialect.

And while they have extensive documentation for their language, several aspects that you run into when dealing with corner cases are at most only defined by "what does the Borland/Embarcadero compiler accept/do". As far as const string parameters, I found this post by Embarcadero's (main) compiler developer: http://stackoverflow.com/questions/5851584/are-const-string-parameters-thread-safe/5851898#5851898

There he explicitly says that const prevents increasing the reference count (and also gives an example program similar to others posted in this thread).

I'm more interested now in a solution. The solutions I've seen so far have potential, but in my opinion most of them seem like they are more complex, would be more overhead, and produce worse performance than just getting rid of the const "optimization".

The main proposals I've seen were
a) treat const string parameters the same as value parameters (i.e., get rid of the current const behaviour)
b) the same as a), combined with a very conservative heuristic to apply the optimization in case there can be no side effects. I.e., in case there is not a single function call and not a single indirect write (var parameter, pointer, global variable, dynamic array, ...) to any string or anything that might be somehow aliased to a string passed as parameter to the current routine
c) keep the current behaviour, but add functionality to the compiler to help debug problems that can occur as a result of problems that can occur as a result of this behaviour

The main problem here is that FPC's exception handling is based on setjump/longjump. This technique has a relatively high overhead for "try", but low overhead when an exception actually occurs (of course, since exceptions are supposed to happen only exceptionally, that's not a really good selling point). The main reason we use it is because it's easy to implement.

A better approach would be to use SEH-based exception handling (which has no overhead at all for "try", and a high overhead in case an exception occurs), but that woud require
a) support for generating EH frames for all platforms (it's currently only supported for a number of i386 and x86-64 platforms) b) support for parsing EH frames on all platforms and performing exception handling based on that

This is definitely something we want, but nobody has found the time yet to implement it.

Jonas

Actually I think you can describe that as 3 independent proposals:

1) Add a compiler switch or directive to disable "const"behaviour for ref counted types (e.g. simply ignore "const" for
string/dyn-array/interface params; Actually downgrade, still prevent assignment to local var)
2) Add automatic optimization of dropping ref-count and exception frame, if the compiler can prove it to be save (e.g. no calls to any other code...). This can happen completely independent of either the present or meaning of const.
3) Add safety checks (similar to range checks)

1)
The only thing I can see, is that future optimization, added to const-param (of all types, e.g also records, etc) can introduce problems, when the value of the const param is indirectly changed, and that may lead to further requests of what const should or should not do. It may be needed to draw a line, and point out that const will always have some dangers. (Apparently with such a "ignore-it-fix" not any more for ansistrings (copy on write), but still for dyn arrays (also ref counted)).
Anyway if the extend off that switch is well defined, it may be worth having. IMHO better as compiler directive, that would allow to put it into the code, and make sure behaviour is stabel (rather than behaviour might change, if a compiler switch is used)

2)
Depends, on someone writing and contributing it. Mostly unrelated to the discussion.

3)
Might not only help to fix issues, but might also create awareness.

For implicit exception (so long as there purpose is only to decrease ref-counts (and free the data if 0 reached), there is another solution.

If an exception frame already exists (even if on a higher stackframe), and the compiler can detect that it does exist:
then instead of creating an exception frame for the current stackframe, all the variables that need refcount decrement, can be added to a list. This list can be processed by the exception handler, before any other code executes. The draw back is, that memory needs to be allocated for this list.

Around a year ago I've experimented with using SEH on Windows (I have not reached a working state back then). It's on my list to revive this again, but before that I want to have Delphi compatible generics and extended RTTI with attributes working :D

Regards,
Sven

Here you are very true. :-)
I had the problem of crashing const UnicodeString parameter in MSEgui too. Heaptrace helped to quickly find the cause.

Martin

> c) keep the current behaviour, but add functionality to the compiler > to help debug problems that can occur as a result of problems that can > occur as a result of this behaviour

Concrete: add means to ignore "const" with managed types.

Further debugging aids as far as feasable. Do we want to make FPC an outstanding compiler, WRT bug hunting? IMO *not* a realistic goal.

+1

2) Add automatic optimization of dropping ref-count and exception frame, if the compiler can prove it to be save (e.g. no calls to any other code...). This can happen completely independent of either the present or meaning of const.

~ [people will point at you if it doesn't work perfectly :-]

3) Add safety checks (similar to range checks)

As a debug feature only, on demand (debugging memory manager?)

Most people don't have problems with const, according debug features are *not* an urgent requirement.

I only agree that a *quick workaround* (global "ignore const" option) is highly desireable (a *must*) for the rare (but real) cases where it can make runtime errors disappear.

Extended debug features are not so important, because code with such problems typically suffers from *general* problems, which suggest a change of the personal coding style. A compiler can prevent an user from shooting himself into one foot, at best, but not prevent him from shooting himself into other limbs :-]

3) Might not only help to fix issues, but might also create awareness.

This may apply to newbies, but IMO will be ignored by people which *believe* that they are superior coders, maybe from practice in other languages. A collection of suggested design patters (resource protection, object ownership...) would be nice - in detail for pointing people with coding style problems to. We could add to that collection all demo code, that forces unexpected exceptions, together with a safe version of the same code, that eliminates the demonstrated problem.

IMO this is what the compiler already does, or should do - an implicit try-finally block with a list of variables to be finalized on exit of a subroutine (used also in stack unwinding after an exception). One should not confuse try-except (*handling* exceptions) and try-finally (guaranteed *cleanup*).

DoDi

I strongly prefer the current situation where the occurrence of exceptions does not incur an important extra overhead compared to the nominal situation.

In the type of applications I write (Air Traffic Control Radar Screens, Electronic Strips and System Interfaces) it is considered good practice to catch exceptions in every procedure because it is a must that the applications always survive and continue to provide the remaining, not affected services.(graceful degradation principle)

I do not want any discussions whether catching exceptions is good or not. If someone thinks otherwise, I do not care at all, do not bother me or this mailing list.

However what is imperative is that when many exceptions occur, there should not be a mayor performance penalty. One of the reasons I use languages such Pascal and Ada for this type of applications, is that they are very fast, (though I also have foreseen code to shutdown services with too many problems in a given time frame). So I beg you to leave exceptions as is (please :-)).

kind regards,

Den Jean

It's a very interesting topic indeed and it's pleasure to read all those comments regardless of the actual consequences :)

Actually when I first reply to the OP, I didn't go deeply in the technical side, but when I finally understood what was going on, a thought (you can call it d to your a,b,c) came. I'm not familiar with all the aspects of the compiler and ansistrings implementation, so my observations can be wrong. So sorry in advance :-)

The answer is indirect referencing. it's a workaround that probably will solve the problem, but I must admit that I don't know what is the exact performance price. The compiler when it detects const s: ansistring could switch to passing not the actual address of the string, but the address of the variable that holds it. In other words passing PString instead of string. In this case no reference counting or exception frame is probably created and at the same time, if the used string is reallocated occasionally because of a side change, the code will not fail because it will just automatically use the new modified address.

Sure I can not implement the exact patch right now but loosely it can be replace with the following approach. It's a simulation with using PString instead of const string. Here are two procedure that I tried to measure

function CRCConstString(const Str: string): integer;
var
  i: integer;
begin
  Result:=0;
  for i:=1 to Length(Str) do
    Result:=Result + Ord(Str[i]);
end;
... measuring
  for i:=0 to 100000 do
    CRCConstString(FString);

...and

function CRCPString(Str: PString): integer;
var
  i: integer;
begin
  Result:=0;
  for i:=1 to Length(Str^) do
    Result:=Result + Ord(Str^[i]);
  end;
end;
.... measuring
  for i:=0 to 100000 do
   CRCPString(@FString);

The bad news is that measuring the difference between two fully optimized version (Windows) gave 20% difference, so PString variant is slower. But I keep being optimistic because:
- Delphi 5 showed no significant difference at all
- if the compiler is doing this, it can do better than me anyway.

Also, I modified the Chad's original example to simulate this apporach and it seems it helped.

Max

function CRCConstString(constref Str: string): integer;

does what you describe

Hmm, it's interesting.. Some observations:
- constref is implemented in 2.5.1, right? Unfortunately I can not test it right now in 2.4.2, it refuses to recognize it, but I I want to test it.
- In LCL it used only with interfaces and TGuid's, no strings or other structures
- there are not much information about constrefs in Lazarus, several posts, the largest part of them is yours, Martin :) so you're probably one of the few who really understand it :)

If constref is what you said , some "quick" fix for those who's scary of the current const behavior could be iintroducing a setting in fpc that forces all const to be constrefs.

Max

Well the LCL can't use it, if it's trunk only (except if IFDEFed). The LCL should compile with release fpc...

See:
http://wiki.lazarus.freepascal.org/FPC_New_Features_Trunk#Constref_parameter_modifier

- See difference to normal "const" is " that it must be passed by reference."

- Note (same as const) it is described as: "This modifier means that the compiler can assume that the parameter is constant" The "assume" means that the requirement for constant-ness goes above that what the compiler can enforce (that is: the compiler can enforce only the local var; but constant implies the value by any means of access)

With what the compiler does today (afaik) none-constantness would not cause any harm (yet). Unless you count it as harm (and you should) that future FPC may (= "is allowed to") compile your code (without any warning or hint) into an exe that behaves different.

Martin wrote

- See difference to normal "const" is " that it must be passed by reference."

I had not read about it before, but I think constref is a huge step in the right direction. It eliminates my fundamental grief with the current implementation.

Specifically, the way const is now "defined" (or not...) is disturbing because it leaves out important details. In C++, you can pass by value or reference, or by pointer which is a value but is used to make references. But what's important is you _always_ know what it's going to do. If you pass something by reference and modify that instance via another reference, they're all going to change, because they are the same instance. And if you pass by value, it's the opposite. The point is, though, you always know what it's going to be.

Imagine if we took C++ and redefined the pass by reference syntax as "the compiler might pass by value or reference, depending what it feels like". Result? Boom. Tons of carefully written code would suddenly break.

It is OK for implementation details to be unspecified. That is how they should be--but only if they are in fact implementation details. If they affect the *meaning* of the program, they have to be defined as part of the language. That's what bothers me. The behavior of const is something that does affect the behavior of the program, but it is undefined. /Usually/ when you pass a record as const it will by by reference, but maybe not. Ditto for other types.

All the problems we've seen with const used in various contexts with different data types--AnsiString, ShortString, records, etc.--are the result of one fundamental problem, which is the language design. Const is not clearly defined. All problems mentioned stem from this.

If we *knew* that const meant it would be by reference, then that immediately eliminates the confusion in the case of ShortString and records; modifying one instance through several references affects them all, as expected. What the programmer says happens; there can be no "bug", except in the programmer's own knowledge.

This precision of language is closely tied with the "testing" approach several people have mentioned. The "testing" approach is that you run the program with the const optimization on, and, if it works, great, and if not, turn the optimization off.

I greatly disagree with this approach. Testing is very important, certainly. But you can only ever test a tiny fraction of real world scenarios. Ultimately you are much better off if you can mentally verify that the program is unconditionally *correct*. That is why I cannot accept the ambiguous const feature, because it is impossible to theoretically prove that the program is correct, which is something I try to do as much as possible.

I applaud the notion of constref. I think it should be used everywhere as long as Delphi compatibility is not required.

The difference is slight. In many (but not all--and that's the key) cases using const or constref would result in the same assembly, if my understanding is correct. What's really important is that it allows the programmer to reason logically about a piece of code without mysteries popping up.

...

We do know:
http://www.freepascal.org/docs-html/ref/refsu58.html#x135-14500011.4.4

A constant argument is passed by reference if its size is larger than a pointer. It is passed by value if the size is equal or is less then the size of a native pointer.

Of course if you want to rely on this, you must ensure the size of your type does ot change (a records definition can be chnaged, and that would have consequences for "const" param). But then, if you rely on it, but some compiler $IF and $FAIL in the code, to ensure it only compiles if your assumptions are met.

Personally I thing this page would be a good place to document the properties of "const" param with regards to ref-counted types...

All the problems we've seen with const used in various contexts with different data types--AnsiString, ShortString, records, etc.--are the result of one fundamental problem, which is the language design. Const is not clearly defined. All problems mentioned stem from this.

IMO you should remove the C specs from your brain first, before you start using Pascal. Pascal "const" parameters are optimization hints in the first place, the write protection of parameters is a less important item instead.

Also C++ references are somewhat different from Pascal references, while both can make your code crash when the referenced object is modified or destroyed during subroutine execution - regardless of const or not.

Pascal ShortString and AnsiString have no counterpart in the C world. If you don't understand how to use these language features, then use PChar instead, to get back your C feeling with strings.

If you are incapable of learning how to write proper code, that allows for "const" optimization, you better keep your hands off it.

When you know that "const" parameters typically are passed by reference, what else do you have to know, that's different from C/C++ behaviour?

DoDi

Thanks for your notes, Martin. I compiled the trunk fpc (2.5.1) and compile the example with constref

function TForm33.CRCConstRef(constref Str: string): integer;
var
  i: integer;
begin
  Result:=0;
  for i:=1 to Length(Str) do
    Result:=Result + Ord(Str[i]);
end;

it worked, also the timing is exactly the same as for PString, so your information (PString = constref Str) is indirectly confirmed. Btw, Chad's example also worked

I understand that currently it works also because as you mentioned the compiler doesn't apply special optimization, sure it could. But also it's possible to forbid it for this special case. Since ansistring is a special entity, not present in COM at all, so with regard to "standards" it can be fully internal decision. Also since there are no consref in Delphi as I suppose, they can't be the authority in this regard :)

If everything it taken care of, apart from possibly global switch to do const = constref aliasing, lcl also can gradually (when fpc 2.5.1 becomes requirement) replace const with constref in critical places (for example when probable events can be fired). Sure this can break inheritance, so just a thought to think about.

Max

Isn't it time to cut this discussion short? const means do not modify! full stop! whatever by reference or value. The rest is bovine extrement.

Thaddy wrote

Isn't it time to cut this discussion short? const means do not modify! full stop! whatever by reference or value. The rest is bovine extrement.

Not so fast.
As Hans-Peter Diettrich said: "Pascal "const" parameters are optimization hints in the first place, the write protection of parameters is a less important item instead".

Is very important, for optimizations, we know if the parameter is passed by reference or value... don't you think?

Marcos Douglas

Wow, thanks for the insults guys. I didn't realize I was so stupid.

You missed my point too, BTW. According to the link given:

"A constant argument is passed by reference if its size is larger than a pointer."

So you always know what the size of a pointer is? If I have this record:

TMyRec = record
   I: Integer;
end;

Is that passed by value or ref? Is someone compiling on 32-bit or 64? You don't know, and neither does anyone. And that is my point about why having it defined is important.

I can't even remember the last time I used C++. My point in mentioning it was only to demonstrate by analogy what could happen if the same ambiguity present in the Pascal construct was present in other well-known languages.

All the argument that such "bug" could exist with shortstrings and records as well is because nobody can say for sure whether they are supposed to be passed by value or by ref--it could be either. With constref, there is no ambiguity. You know it will be by ref, hence there is no question as to whether it's a bug. Otherwise it's just a matter of opinion.

I guess my brain is way to tiny to get past the notion that a "write once, compile anywhere" programming language should be defined in such a way that the meaning of the program doesn't spontaneously change depending on the compilation platform.

Martin wrote

We do know: http://www.freepascal.org/docs-html/ref/refsu58.html#x135-14500011.4.4 > A constant argument is passed by reference if its size is larger than a pointer. It is passed by value if the size is equal or is less then the size of a native pointer.

That part of the manual is wrong. What const does is by design completely implementation-dependent (except for cdecl/cppdecl routines, where it behaves the same as in C/C++, and for mwpascal routines, where it behaves the same as in MetroWerks Pascal). And yes, this is different from the meaning of const C and C++ (which is why the meaning is specified separately there). And yes, this means that it can be unsafe if you pass global variables as const and the modify them in the called routine, and that every objection voiced already 10 times in this thread applies. That is how const has always worked since it was introduced in Turbo Pascal. And no, that does not mean this is ideal, but rewriting the entire const handling is not going to happen.

Alternatives (such as constref) can be used, and checks can be added, etc, but in general const has always meant in FPC (and Delphi) and will probably always mean "I promise that I will not modify this parameter in any way, please help me as far as you can in preventing accidental modifications, and apart from that you can generate any code you want based on that assumption".

Jonas

Jonas Maebe wrote

That part of the manual is wrong. What const does is by design completely implementation-dependent (except for cdecl/cppdecl routines, where it behaves the same as in C/C++, and for mwpascal routines, where it behaves the same as in MetroWerks Pascal). And yes, this is different from the meaning of const C and C++ (which is why the meaning is specified separately there). And yes, this means that it can be unsafe if you pass global variables as const and the modify them in the called routine, and that every objection voiced already 10 times in this thread applies. That is how const has always worked since it was introduced in Turbo Pascal. And no, that does not mean this is ideal, but rewriting the entire const handling is not going to happen.

And yes, that should all be mentioned in the manual.

Jonas

"Native pointer size" is exact term, because size of native pointer on each platform is known and fixed (will not change in the future). I don't know all of those sizes, but that is whole other point, and should be (if needed at all) fixed in documentation, not language.

I can't even remember the last time I used C++. My point in mentioning it was only to demonstrate by analogy what could happen if the same ambiguity present in the Pascal construct was present in other well-known languages.

It's not that easy to compare Pascal const with C++ const, because in C++ there are other const language elements: const variable, const function result, const object method, const object field (on top of my head).

Just to mention that C++ suffers from the same problem as Pascal on this matter. C++ doesn't have built-in refcount-ed types, but if you use refcount-ed types, you need to take care of the exactly same thing as in Pascal - you have to choose: either to enable refcount-ing (and make your code safe and slower), or disable it (and make your code faster but possibly unsafe).

As mentioned above, behavior of compiler is very well defined. If you use var or constref, parameter is passed by ref. If you use no keyword, parameter is passed by value. If you use const, it's compiler's (well defined) decision.

I guess my brain is way to tiny to get past the notion that a "write once, compile anywhere" programming language should be defined in such a way that the meaning of the program doesn't spontaneously change depending on the compilation platform.

Just checked

http://www.freepascal.org/docs-html/ref/refsu58.html

and it doesn't mention optimization of const refcount-ed parameters (and possible problems), but Michael already said he plans (if I followed discussion correctly) to update documention on this matter.

On the other hand, that page explicitly mentions that it is compiler's decision weather const parameter will be passed by value or by reference. Whatever choice compiler makes, your program will not work (only) if you make semantic mistake (in problematic corner case, your program will either crash ugly, or it won't work correctly).

Maybe Pascal needs 'constval' keyword? It would always pass parameter by value, but it would reject assigning new value to this parameter.

Another (possible) solution is to add new behavoir to const refcount-ed parameter, with the following rule:

- When local variable or local const parameter is passed to some function's const parameter, optimization takes place, and there is no increase of refcount. In all other situation (when you pass object's variable, global variable, non-const parameter), refcount is increased on the caller side.

Regards,
Aleksa

On Fri, Jul 8, 2011 at 7:05 AM, Jonas Maebe wrote: (...)

(...)

Isn't this unfortunately encumbered by patents?
http://wiki.winehq.org/CompilerExceptionSupport
http://yro.slashdot.org/story/05/05/12/1947213/Winelib-Hobbled-by-Exception-Handling-Patent

-Flávio

Thanks for ignoring half of my mail. If it matters to write code depended on the rev/value question, use $IFDEF. I did point that out.

You did ask, if it was defined: it is defined. Full stop. You do not like the way it is defined, <and I stop the sentence here, for i do not know how to finish it without insult>

Op Sat, 9 Jul 2011, schreef Flávio Etrusco:

Isn't this unfortunately encumbered by patents? http://wiki.winehq.org/CompilerExceptionSupport http://yro.slashdot.org/story/05/05/12/1947213/Winelib-Hobbled-by-Exception-Handling-Patent

That is a problem. However:
* It's a US patent, so irrelevant to FPC developers
* Having a patent # in the US means nothing, the patent gain value once it is confirmed by court
* Borland said it the past they were open for discussion to allow it to be used in open source compilers (though not specfically talking about FPC)
* It's a patent from 1994, so almost expired.

Daniël

Wow, thanks for the insults guys. I didn't realize I was so stupid.

Waking up? ;-)

You missed my point too, BTW. According to the link given: "A constant argument is passed by reference if its size is larger than a pointer."

People like you should not read such detailed explanations, which they tend to misinterpret :-(

NACK. Most languages have some vague specifications, like the size of int or Integer. This is *intentional*, and every coder should write his code in a way, that it works under all allowed circumstances.

I guess my brain is way to tiny to get past the notion that a "write once, compile anywhere" programming language should be defined in such a way that the meaning of the program doesn't spontaneously change depending on the compilation platform.

It's up to YOU to make your code behave properly on every platform. If you assume e.g. a specific byte order, your code can crash on any other platform, regardless of the programming language you choose.

DoDi

Isn't this unfortunately encumbered by patents? http://wiki.winehq.org/CompilerExceptionSupport http://yro.slashdot.org/story/05/05/12/1947213/Winelib-Hobbled-by-Exception-Handling-Patent

Software patents should not be a problem outside the USA. A nice try, but unimportant for non-resident free software writers and distributors.

DoDi

I know that, I just wanted to raise the issue, since nobody raised it and I don't/didn't whether all FPC developers lived outside US. I'm brazilian (and live in Brazil) and this is - for now... - not an issue here either.

-Flávio

Martin, I do not know why you and Hans suddently have an urge to insult Chad, but he really did not deserve this.

As for the whole "optimization hint" angle, I'd like to note that:
1) This is contrary to previous posts in this same thread, where FPC developers insisted that "const" semantics is defined as "no refcounting", which is quite different from a hint.
2) If "const" is indeed an optimization hint, that places it in the same category as,
say, "inline". What would you say if adding "inline" keyword to a procedure converted working program into randomly crashing one? I'd say this is optimization bug,
much like the title of this thread.
3) Current documentation (http://www.freepascal.org/docs-html/ref/refsu58.html) declares that "const" modifier is "retaining the semantics of passing by value",
so there is definitely a bug here -- you can declare it to be bug in specification, but still.

On the more constructive note, I have yet another proposal:
1) Leave current "const" implementation broken as-is.
2) Introduce new "constval" modifier which is similar to "const", but guarantees correct "by-value" semantics.
3) Slowly migrate existing code to either "constref" or "constval", use "const" for legacy/compatibility/extreme optimization cases.

Martin, I do not know why you and Hans suddently have an urge to insult Chad, but he really did not deserve this.

Perhaps a threshold was reached, when the same wrong argumentation was refrained too often.

As for the whole "optimization hint" angle, I'd like to note that: 1) This is contrary to previous posts in this same thread, where FPC developers insisted that "const" semantics is defined as "no refcounting", which is quite different from a hint.

Refcounting is one optimization aspect, passing large data byref another one.

IMO the situation is very different for "inline". Both optimizations have very little in common (from the compiler VP).

Sorry, I cannot find anything like this statement in the link. The addressed byval/byref move doesn't change the semantics, provided that the item is not modified in user code - this can be verified for records or other simple types by the compiler.

The situation is very different for parameters, which already *are* references. In these cases the passed object can be modified, regardless of "const", the compiler only checks that the reference *itself* is not changed.

The situation is different again for *managed* types, which also are always passed byref. Here the removal of refcounting applies.

On the more constructive note, I have yet another proposal: 1) Leave current "const" implementation broken as-is. 2) Introduce new "constval" modifier which is similar to "const", but guarantees correct "by-value" semantics.

If you want to introduce different keywords, then please different for
beforementioned cases. E.g.
constval x: integer; //all value-types
constref r: Trecord; //also ShortString
const??? o: TObject; //all references, except following
const??? s: AnsiString; //all managed types

BTW the "constref" keyword has been introduced for *external* subroutines (MacOS ABI), which require that a value is always passed by reference, independent from SizeOf(param) and SizeOf(pointer). Its use should be restricted accordingly.

3) Slowly migrate existing code to either "constref" or "constval", use "const" for legacy/compatibility/extreme optimization cases.

<IMO> "const" is already used far too often in the FPC/Lazarus libraries. The compiler instead should treat it as an error, when an unmanaged (object) reference is used with "const".

It's kidding when somebody requires that the compiler prevents him from inadvertently modifying an given value parameter. He who doesn't have enough coding discipline to care for such restrictions himself, will make more logical errors that no compiler can detect.

When the meaning of "const"/"constref" is documented as kind of a *calling convention*, that allows the compiler to optimize the code, no more discussion is required.

</IMO>

DoDi

Yes, but the argument was made that "no refcounting" is the required feature of const, not merely an "optimization aspect".

Having no common implementation code does not prevent those features from being similar from user's POV.
BTW they are not as independent as you might think -- did you know that adding "inline" currently changes semantics of "const"?

It is a direct quote -- you can use "find on page" feature of your browser (usually activated by pressing Ctrl+F) to locate it.

I am not quite sure what are you asking here. Can you elaborate?

BTW the "constref" keyword has been introduced for *external* subroutines (MacOS ABI), which require that a value is always passed by reference, independent from SizeOf(param) and SizeOf(pointer). Its use should be restricted accordingly.

Why? It is well-defined and useful construct, certainly better then "const".

> 3) Slowly migrate existing code to either "constref" or "constval", >  use "const" for legacy/compatibility/extreme optimization cases. <IMO> "const" is already used far too often in the FPC/Lazarus libraries.

Totally agree.

The compiler instead should treat it as an error, when an unmanaged (object) reference is used with "const".

Good idea, BTW! Probably error is out of question for compatibility reasons, but a warning would certainly be good.
FPC developers: maybe I'll create feature request for this? It is quite independent of the rest of thread.

When the meaning of "const"/"constref" is documented as kind of a *calling convention*, that allows the compiler to optimize the code, no more discussion is required.

"constref" is a calling convention, but "const" is not, since it is "optimization hint".
As you said above, this is the reason why "constref" was invented in the first place.

In our previous episode, Alexander Klenin said:

This sentence is the main problem. If you define "working" as any code that happens to run must remain working, you are moving away from building a compiler, and starting to build an emulator.

A compiler produces a defined output for a defined input. But the input is defined as adhering to the language rules.

An emulator produces a defined output for a set of inputs (existing programs). The input is defined as is.

Of course in a project the size of FPC not every corner case is documented to the last detail, so the rules are made up as we go. And of course the frequency of the problem has a say in it too.

To be honest, I'm a bit surprised by the progression of this discussion. I sympathise a bit with the proponents of a simple "disable this optimization" switch, but to me the rest all sounds like people desperately defending their turf, and losing sight of the big picture.

Specially since I already discussed this issue a bit with Martin before the email discussion began.

The main argumentation for this switch for me is that in many cases programmers don't have complete control of the codebase (yet). Parts are inherited from previous employees, taken of the net or simply bought.

In such cases remarks like "you shouldn't this" are totally useless since you inherited a mountain of code, which will take years to comb through.

If you then have strange crashes, a simple recompile with such switch might give you some information if such problem is happening, or not, somewhere deep in your framework.

Just like you compile once with -gtt with -gh, run it through valgrind etc etc.

I think the finegrained detection option that Martin proposes has a bad work vs occuarance tradeoff, but a global killswitch for this might be enough.

Although I liked constref at first, I'd rather think that the current behavior is more of a side effect of the current implementation. I see that it was introduce to support well-known c/c++ concept of "give me a pointer to the thing and I promise I won't change the thing" used in many apis. But since ansistring is not used in any universal api, the compiler currently is free to change the meaning and for example pass not the address of the entry that hold the pointer, but pointer itself. In other words the compiler if free to choose because lazarus/fpc is the only place on earth where the pair constref-ansistring exists (currently). So the constref immunity is just a side effect

And returning to the original discussion currently I'd like to look from another point (or maybe just summation of previous thoughts of others). I think that anything passed by reference (pure reference) pose the problem OP described, even if you use current "immune" constref. For example, when you want to investigate every char of the string you will probably use for i:=1 to Length(Str). As I recall taking length in advance and using it for every step is even the part Wirth's pascal as the requirement. If so, during the loop a callback or just some call shorten the string, the loop will fail almost for sure. Not because unexpectedly the string is invalid, but because the loop expects the string to be longer that it is now. And the problem like this also can be hard to detect.

So personally I'd not separate the reference-based problems like this from the problems posed by const. We can blame the compiler for some optimization results (rarely), but we can not blame the programmer who cached the length of the string in a local variable thinking that the string will be same till the end of the function. And imho understanding that referencing can be dangerous in this regard should be a part of knowledge for every programmer.

Some good news )... If we look at this, we will see that the problem exists for every data passed by reference in any language. But in contrary to some languages we have a luxury of passing by value not only register-sized data, but also arge data. And to make things even better we have a luxury of having a tricky ansistring with its copy-on-write concept. So for those who appreciate this luxury and afraid of reference-based problems, using passing ansistrings by value is the answer. Remember, it's a luxury ;)

Max

Jonas himself pointed out that this documentation is at least misleading.

Anyway: I have at no point said or implied (or at lest have I not mean to do so) that the documentation was perfect. In fact, I do thing the documentation has severe shortcomings.

Anyway, so far I have talked about the feature. The feature is correctly implemented (if the defintition is known).

I guess an inline can make a program crash. I the inlined procedure contains code trying to access it's own stack frame, and being inlined actually access the stackframe of the would-be-caller... I am not going to try to prove this, but I guess with enough determination it can be done.

Anyway, as we well know by now "const" is more than that, hence it can not be compared with inline.

...and never mind any issues with the documentation of it. If the docs are wrong, then correct them, but do not blame the feature as bad, because the docs are bad...

No argument with that. As I pointed out, the correctness (or incorrectness) were never my point

On the more constructive note, I have yet another proposal: 1) Leave current "const" implementation broken as-is.

<sarcasm>
You made a typo:
1) Leave current "const" implementation WORKING as-is. ;-p SCNR
</sarcasm>

2) Introduce new "constval" modifier which is similar to "const", but guarantees correct "by-value" semantics.

no problem with that.

3) Slowly migrate existing code to either "constref" or "constval", use "const" for legacy/compatibility/extreme optimization cases.

Maybe...

BUT: I have actually used "const" in some places with exactly the intend to have it doing what it does.
I have used const with the very intention of the ref-count not being increased, and by this with the effect of the implicit exception frame not being needed.

Given that, I would not migrate that code to anything else.

BTW: when I first did that, I made the very mistake of doing it wrong myself too. I learned, I corrected my code, I started using it as it is meant to..

Best Regards
Martin

I agree the discussion, if the feature should be as it is, went off, far out off control. And I myself take a considerable share of the blame in making it do so.....

The discussion was meant to be about, what can or should be done, in addition to the feature. Accepting the feature itself as it is. I think the finegrained detection option that Martin proposes has a bad

work vs occuarance tradeoff, but a global killswitch for this might be enough.

I do agree. At least from the point of the developer(s) that have to implement it. And I do accept that.

Never the mind, that does not mean that not from the users point of view, it would be nice to have. not that it cannot be proposed.

To any datatype, of course. The result should be:
1) Parameter is passed by value
2) Parameter modifications are forbidden by compiler

The whole sentence applies to the byval/byref modification, nothing is said about the cases where references are passed even without const. In this case I agree that aliasing can change the semantics.

To which data type do you want apply your "constval" modifier, with which concrete effects?

From the user VP both affect code generation only. Both can be broken by by aliasing.

DoDi

Some thoughts on the meaning of const, constref, and "constval", and how they can usefully be applied:

My initial understanding of const was hazy. I have come to appreciate that it is defined, but only in a very loose way. Instead of undefined I should say ill-defined. I'm not entirely sure what some of the attacks have been about, but I will acknowledge that I was not 100% sure about const when I started. What I have said about it being unclear is certainly true though. Let me explain how, and what I think a better idea would be.

I should not say const is undefined. Actually I mean const is ambiguous; sort of defined as undefined, in that: 1) with the current definition, it changes by
platform, 2) the current definition is wrong anyway, and 3) it is implementation dependent, which I interpret to mean it could change in the future.

If the meaning of const is "entirely implementation-dependent", and if I interpret that correctly, I think we really cannot assume anything about what exactly will happen with regard to the calling convention, which is what I was worried about earlier.

Now, constref was intended for compatibility with other languages, but I think it is a valuable addition to Pascal in its own right and that it would be wise to use.

The hypothetical constval modifier is a natural corollary to this. In the current implementation, if you were to pass a record to a procedure for example, you have two choices: 1) pass it by value but have it be non-const, 2) pass it as const, which might be by reference or value. With constval and constref you would have the other two highly useful choices: 1) pass by reference and it's const, 2) pass by value and it's const. Without making assumptions about const, which I don't think we can, these two options do not currently exist, so I think these new keywords are a useful addition.

Some might say that it doesn't matter whether it is passed by value or reference: if you pass it as const, either way you promise not to modify it. See, that is my concern: what is IT? People have used other words, but it ultimately comes down to: if you say the programmer promises not to modify the thing passed as const, what thing *exactly* is that? A variable, reference, or instance? With const, we don't really know. With constref, it means you promise not to modify the memory location (instance) pointed to by that reference. With constval it means that you can change the memory location/instance that was passed into the procedure, since the procedure is now using it's own copy anyway; you just can't modify the instance that the procedure now has it's own copy of.

One additional problem does arise. Constval implies that the implementation is pass-by-value. However in many (I'd say most) cases it is quite possible that we could be interested only in the semantics of pass-by-value, not the implementation. So for AnsiStrings, neither constref nor constval would be suitable. Constref would mean it must be by reference, but we want by-value semantics. Constval would mean that the string has to be copied to a separate memory location, which we might not really care about, and is slow and wasteful.

So, I propose: don't have constval literally mean it is passed by value, i.e.,
pushing the string onto the stack or copying it to a new memory location. Instead, have constval defined as by-value *semantics*. In other words, constval would indicate the meaning of the language, not the implementation. It would not be a calling convention. It would not mean the call would be by value; it would mean the semantics would be by value. This is essentially what I originally thought const would mean with AnsiStrings, though it turned out hazy.

The programmer must know the language and the compiler must implement the language. How the compiler does that should not determine how the program behaves. If constval is defined in semantics, not implementation, this means the compiler can still take whatever optimizations are possible as long as they do not break those semantics. So you would not have to actually copy strings, you'd just have to be sure the semantics are valid. In the current implementation, this would simply mean eliminating the "const optimization" to which I originally objected.

See regarding const:
http://www.freepascal.org/docs-html/ref/refsu58.html

The main use for this [const] is reducing the stack size, hence improving performance, and still retaining the semantics of passing by value...

(Thanks for the link Alexander!)
This is undoubtedly the most conclusive statement I've seen regarding this issue. If this documentation were correct, my initial claim that there is a bug would be correct. However I think we're long beyond that. Instead it seems to be consensus that the documentation is incorrect and the implementation will not change. In that case there is still a need for the functionality that is suggested in that statement.

Up to this point, my observations and proposals:

1. There has been a grand conflation of implementation and semantics.

2. Const is implemented correctly right now. Although defined, the problem is that it is
ill-defined: you can't be quite sure what it's going to do. If you're in a situation where you don't need to know whether it's byval or byref then it's OK, but most of the time that's not good enough. Changing the implementation of const is too risky anyway.

3. Constval should be created and it should represent semantics, not implementation. In other
words, it is not a calling convention. It does not stipulate how parameters would be passed. It is a construct of the language. As long as the compiler preserves its meaning, it's OK. In other words, pass-by-value
semantics, not pass-by-value implementation. When it comes to implementation, which I stress is a separate issue, it would really come down to removing the const optimization as I originally proposed.

4. Constref is already a calling convention, an implementation. Fortunately, it automatically carries the requisite semantics with that implementation, so it's OK as is.

I think Alexander's suggestion is the best I've heard:

Slowly migrate existing code to either "constref" or "constval", use "const" for legacy/compatibility/extreme optimization cases.

And to summarize the discussion:

1. I initially believed const AnsiString parameters had by-value semantics.
2. The quote in the FPC docs supported this.
3. However it seems that most people agree that those docs are wrong.
4. Most of the discussion has freely mixed semantics and implementation
5. The more reading I have done and the more examples people have given of the behavior of records and shortstrings, the more I have realized that you can't really make many assertions about how const will behave. It's more complicated and convoluted than I thought.
6. Const is defined. If you don't care about byval or byref semantics, then it is sufficient. For most cases that's not good enough, in my opinion, and comparison with many other languages will show that other people have realized this long ago.
7. Constref is a great addition. Although it is defined in terms of its implementation, that implementation automatically has the necessary semantics, so it is good as-is.
8. Constval, I propose, should be defined as pass-by-value semantics. This means the implementation could still be pass-by-reference with reference counting and copy-on-write. It would just mean the compiler is not free to violate pass-by-value semantics as it currently is permitted to do.

Op Sun, 10 Jul 2011, schreef Chad Berchek:

I think we really cannot assume anything about what exactly will happen with regard to the calling convention, which is what I was worried about earlier.

Within a certain calling convention, the behaviour of const is defined and will not change. I.e. an int64 may be passed by value in one calling convention and by ref in another, but once the calling convention is fixed, future compatibily is maintained; your assembler routines can safely assume the behaviour won't change.

However, for a new calling convention (on an existing or a new platform) it's completely up to the compiler designer what will be passed as const and what will be passed as reference.

You Pascal code should be able to handle both value *and* reference. After all, with const you are leaving the decision to the compiler. This is what it means in the end. Then your Pascal code should not depend on specific behaviour.

Daniël

Actually, even that is not true: http://wiki.freepascal.org/User_Changes_Trunk#Const_parameter_passing_on_non-Win64_platforms

const parameters are indeed not suited to assembler routines for that reason.

Jonas

Op Sun, 10 Jul 2011, schreef Jonas Maebe:

Calling conventions should in principle be eternally frozen. This includes const, because for exampe a DLL compiled in version x can be imported by a program compiled by version y. The fact that it has happened (for whatever reason) does not mean it is good practise.

Daniël

Daniël Mantione wrote

Calling conventions should in principle be eternally frozen. This includes const, because for exampe a DLL compiled in version x can be imported by a program compiled by version y.

That's indeed a good point.

The fact that it has happened (for whatever reason) does not mean it is good practise.

True.

Jonas

Daniël Mantione wrote

However, for a new calling convention (on an existing or a new platform) it's completely up to the compiler designer what will be passed as const and what will be passed as reference.

Agreed. That's why I say you can't really make assumptions: if you assume a certain platform, you can make a limited set of assumptions (very limited as I will show in a moment). However one of FPC's great features is its cross-platform power, so I don't want to make assumptions about the platform. Without assumptions about platform you can make even less assumptions about const.

Within a certain calling convention, the behaviour of const is defined and will not change.

Even if the calling convention does not change, the semantics can, as currently implemented. Even within a single platform const is ill-defined. Consider the case of AnsiString. There are three cases as it is currently implemented:

1) Ref count > 1 when passed as const parameter results in strict pass-by-value semantics
2) Ref count = 1 and you modify the original instance in-place results in the value of the parameter changing
3) Ref count = 1 and you modify the original reference such that the instance's ref count decreases to zero and it may crash or just give weird behavior

Although the programmer knows about reference counting, it is not expected for the programmer to know the details of the implementation, which can change. Therefore you cannot be 100% certain what the refcount is at any given time. And therefore you cannot be certain how const will be treated. We know the implementation will be pass-by-ref with no refcount change. The semantics could be pass-by-value, pass-by-ref, or just plain crashing, depending on details we cannot predict.

You Pascal code should be able to handle both value *and* reference. After all, with const you are leaving the decision to the compiler. This is what it means in the end. Then your Pascal code should not depend on specific behaviour.

Yes, I agree fully. That is the only choice with const. Constref, as it exists, with the addition of constval, provide a more precise form of expression on those occasions where one might feel the ambiguity of const is insufficient.

And before anyone says something, yes, I also understand that if you pass something as a const, you have to be prepared in case it is passed by reference, and you have to realize that if it is passed by reference, you have to promise not to modify the instance via another reference. That's OK.

I am a relative newcomer to Pascal, having used it about 1.5 years now. It was disturbing that something so important was not documented anywhere, and, in fact, there was documentation to the contrary. Aside from the lacking and sometimes incorrect documentation (I refer not only to FPC but to Delphi), I was surprised when the semantics of const AnsiStrings, which usually turn out to be pass-by-value, actually might not be in rare cases that are hard to predict.

Aside from good documentation (which Delphi is in need of too in this particular case), I am proposing constval in conjunction with the existing constref to provide greater expressive power and precision for the programmer.

Sorry for the additional post; I should have added this to my preceding message.

I just want to clarify: I am trying to be very careful to distinguish between implementation and semantics.

If my understanding is correct, AnsiString implementation is always pass-by-reference.

The problem is the programmer should be forced to stick to certain semantics, not a certain implementation. With const AnsiString, the implementation is consistently pass-by-ref, but the semantics can change as described in my previous message, depending on the instance's refcount when the procedure is invoked and what you may do with it while the procedure is running.

It has correctly been pointed out that you therefore can't make an assumption about what exactly a const AnsiString parameter will do; you just have to be prepared in either case, and if it turns out to be by reference, don't modify the instance via another reference.

I simply propose constref (existing) and constval (hyptothetical) for those who come across situations where more precise programming would be desired.

--- Implementation ---
I had a few ideas about implementation of constval, as I described it, for AnsiString. (Or, alternatively, a way to change the behavior of const, though this is no longer what I advocate.) These are all speculative. These are some ideas, basically, not assertions. Also my knowledge of how FPC works is limited as I've said before.

1. It seems that if a string is a (non-const) local variable it should be safe. I base this on the following reasoning:

a) In order to trigger the undesired behavior, you have to get an instance with a refcount of 1 which actually has more than one reference to it. The only way to do this is to pass a reference to an instance with a refcount of 1 to a function accepting a const AnsiString parameter. (Aside: The critical value is 1 because that determines whether copy-on-write happens and whether it gets freed the next time the refcount is decremented.)
b) Furthermore, you must be able to access the non-const reference at a higher scope (object, class, or global).
c) To do this, you could either have a higher-scope variable which you assign to a local variable, or vice versa.
d) If you do either of these, the refcount becomes 2 and the problem cannot occur.
e) The problem cannot be triggered passing a local variable.

2. Implementing 1 would require that the reference count update can be applied or not applied to a specific function depending how it's called. This could be done in two ways I'm thinking of:
a) Move the responsibility for updating the refcount to the caller. There are pros and cons to this idea. It could result in slightly larger code (because the refcount update and try-finally are in more places). However I think it could also speed things up *even more than the current implementation* because the refcount updates for several non-const strings could be combined into a single try-finally in the caller, rather than having one in each function.
b) Another possibility is to have two entry points to the function. The update of the refcount and try-finally would remain in the function, not the caller. However the caller could enter the function at either of two entry points. One would do the refcount update and set up the try-finally, whereas the other would skip to the code after that. I don't know if this is even possible. It seemed like something that might be doable with some tweaking when I was looking at the assembly code.

Chad Berchek wrote

1. It seems that if a string is a (non-const) local variable it should be safe. I base this on the following reasoning:

looks like it's not:

procedure TForm1.Button1Click(Sender: TObject);
var
  S: string;
  procedure SideEffectCall;
  begin
    SetLength(S, 0);
  end;
  function CalcSomething(const T: string): integer;
  begin
    Result:=0;
    SideEffectCall;
    MessageDlg(T, mtInformation, [mbOk], 0); // crash here
  end;
begin
  S:='12345678901234567890';
  S:=S + S;
  CalcSomething(S);
end;

How to pass objects by value?
Records with embedded AnsiStrings or other managed types?

DoDi

IMO we know that then the data, passed as immediate values on the stack, shall not be modified by the subroutine. The only confusion was about const records (and ShortStrings?), when passed by reference. In this case the content of the referenced record cannot be changed.

When alias are of concern, don't use "const".

With constref, it means you promise not to modify the memory location (instance) pointed to by that reference.

Quite useless with objects, which typically are modified by method calls. Otherwise same as above.

IMO that's the normal (non-const) procedure, so far. Any changes to the passed arguments are invisible after exit from the subroutine.

That's why copy-on-write was invented. As soon as the data is modified, a (local) copy is created.

IMO that's such a weak definition, that it gave birth to this never ending thread :-(

In the tradition of Wirth, it should be possible to describe the language semantics by a few implementation details. The implications *resulting* from the implementation, like alias, are already discussed in common literature.

Aliasing is not considered a bug, it's a fact. When a pointer is passed to a subroutine, it should be clear to every coder that the referenced memory *can* change during the lifetime of the pointer, even if the pointer (address) is never changed; the same for all references. When a coder allows to pass a data structure byref instead of byval, by using "const", he'll have to accept all the consequences. Since this difference was the only rationale for introducing "const" into the language, it has to be accepted as defined, and the documentation should be updated accordingly.

Effectively the "...semantics..." statement should be dropped from the docs, and instead the effects of the "const" modifier on *all* affected datatypes have to be added.

DoDi

The compiler has to implement correctly the ABI of every platform, for compatibility with external functions. Assembler code has to be rewritten for every target machine and ABI.

The language can only specify proprietary calling conventions (ABI), that are bound to specific processors, so that assembler code can be written once for every processor, regardless of the platform ABI.

The use of "const" parameters and generic types is nonsense with assembler routines, because these are compiler specific implementation details. I'd also ban their use in shared libraries (DLL/so), which must have an immutable ABI. Here's another place for the "constref" modifier.

DoDi

I just want to clarify: I am trying to be very careful to distinguish between implementation and semantics.

"Implementation" is a very precise definition, which can be used to perfectly describe the semantics of a language. A verbose description of the semantics leaves too much room for misunderstandings, and commonly accepted formalisms are at least as hard to understand as implementation details, while not covering all aspects.

DoDi

Replace it by

   S: TObject;

and
S.Free
to see that you can force any undesireable effect with reference types by alias.

DoDi

Case 1) is the same as 2) and 3). Even if the reference count is initially 100, the called routine can overwrite all but one of them first and then either do 2) or 3).

Jonas

That, as I said above, is a separate good question. Until it is resolved, I agree with your suggestion of giving warning (or even error) on "const" and "constval" object parameters.

Records with embedded AnsiStrings or other managed types?

That is already done in the usual by-value mechanism.