Compiler/Object Pascal Bug!!!???
Davor Barcan
Windows, Messages, SysUtils, Classes, Graphics, Controls, Forms, Dialogs;
TForm1 = class(TForm)
procedure FormCreate(Sender: TObject);
private
{ Private declarations }
public
{ Public declarations }
end;
public
procedure DoIt (Value: integer);
end;
var
Form1: TForm1;
var
List: TList;
i: integer;
var
j: integer;
begin
j := Value; // Blows up really nice (or really weird value)!!
end;
List := TList.Create;
List.Add (Pointer (1)); // Add a couple of items, so we have
List.Add (Pointer (2)); // something to sort
List.Sort (@SortCompare);
finally
List.Free;
end;
end;
procedure TForm1.FormCreate(Sender: TObject);
var
Test1: TTest;
begin
Test1 := TTest.Create;
Test1.DoIt (123); // Call our example method
Test1.Free;
end;
Hallvard Vassbotn
>I wrote some code that used TList.Sort to sort a number of records. The comparison function (SortCompare in this example) accesses another variable that is supposed to be within scope; TList.Sort only passes two parameters, so I grab other info from elsewhere.
>
>The problem is that it usually crashes!!
This is due to a bug in your code. Not Borland's.
>procedure TTest.DoIt (Value: integer);
>
> function SortCompare (Item1, Item2: Pointer): Integer;
> begin
> j := Value; // Blows up really nice (or really weird value)!!
>
>begin
> List.Sort (@SortCompare);
The compare function you send to TList.Sort must be a global function,
not a local nested function as you have here. Borland's design here is
questionable, because there is no way to send a private cookie-value
to this global function so if you need to communicate some value(s) to
the function, you _must_ use one or more global variables for this.
Not very elegant, but that's how it works.
Move your SortCompare outside the DoIt method. Then remove the @
operator in the List.Sort call.
--
Hallvard Vassbotn
Falcon R&D, Reuters Norge AS
Chris Roberts
"Davor Barcan" <davor....@cybersoft.ca> wrote:
Hi Davor. Please turn off the HTML. It triples the size of an article.
(BORLAND NEWSGROUP GUIDELINES parts 10 & 11)
>I wrote some code that used TList.Sort to sort a number of records. The =
>comparison function (SortCompare in this example) accesses another =
>variable that is supposed to be within scope; TList.Sort only passes two =
>parameters, so I grab other info from elsewhere.
>
>The problem is that it usually crashes!!
>
>I wrote a very simple example to illustrate this. Just create a form =
>with a FormCreate method. The code is very short and uses one other =
>class called TTest. This class only has one method which illustrates =
>the bug.
>
>For some reason, the debugger thinks that List, i, and Value are all =
>inaccessible within the SortCompare function, and who knows what the =
>compiler is thinking because it compiles the code fine!
There's no reason why the compiler wouldn't compile the code that I can
see. A standard local function referencing Value is OK. However,
SortCompare is referenced by address. Must be something to do with
that???
Regards,
Chris Roberts
Davor Barcan
Sorry for making my original post in HTML ...
>>procedure TTest.DoIt (Value: integer);
>>
>> function SortCompare (Item1, Item2: Pointer): Integer;
>> begin
>> j := Value; // Blows up really nice (or really weird value)!!
>>
>>begin
>> List.Sort (@SortCompare);
>
>The compare function you send to TList.Sort must be a global function,
>not a local nested function as you have here.
There is no way (at least in Object Pascal) that you can enforce that.
Scoping rules are the only way to limit function visibility. The compiler
should not care; a function pointer is a function pointer.
In fact, other than not being able to access the proper stack frame (and
therefore the variables Value, list, and i), this code works fine. I have
successfully used a local function many times as a value to TList.Sort.
But, just to make sure. I changed the code as follows:
- I made Value a global variable that gets initialized somewhere.
- Removed the parameter from DoIt and changed the reference to it: procedure
TTest.DoIt
The results? When you run the code this time, it works fine. Although you
still cannot get valid values for List and i within the SortCompare
function, the now global variable, Value, shows up correctly.
This seems to be suggesting a *stack frame problem* that the compiler is not
handling.
>Move your SortCompare outside the DoIt method. Then remove the @
>operator in the List.Sort call.
But if you don't use the '@', the compiler will try to execute the
SortCompare function instead of passing it.
Comments?
Davor Barcan
bar...@cybersurf.net
Hallvard Vassbotn
>There is no way (at least in Object Pascal) that you can enforce that.
>Scoping rules are the only way to limit function visibility. The compiler
>should not care; a function pointer is a function pointer.
Not exactly. When calling a nested function, the compiler does some
magic to let the nested function see the variables declared in the
outer scope. This is most often implemented by pushing a stack frame
pointer (BP/EBP) before calling the nested function.
The code in TList.Sort expects the parameter to be a valid function
pointer to a global routine, not a nested local function, so it does
not setup this extra stack frame and thus your SortCompare cannot see
the outer-scope variables at run-time. The code in SortCompare was
compiled with the assumption that this stack frame was setup correctly
by the outer-function. So the code crashes when that is not the case.
There is no way in Object Pascal to declare a pointer to a nested
function. Old timers will remember the TV/OWL methods ForEach,
FirstThat et. al. These were actually implemented with local functions
in mind, but to implement them Borland had to use assembly language.
Even in VCL this technique is used, albeit only in the implementation
part of the Grids unit (if you have the source code: search for
TSparsePointerArray.ForAll).
>In fact, other than not being able to access the proper stack frame (and
>therefore the variables Value, list, and i), this code works fine. I have
>successfully used a local function many times as a value to TList.Sort.
Yes, but it blows up at run-time if you accidently references any of
those outer-scope variables. It is much safer to make the function a
proper global function (i.e. declared in the outer-most scope).
>This seems to be suggesting a *stack frame problem* that the compiler is not
>handling.
Yes, see above.
>>Move your SortCompare outside the DoIt method. Then remove the @
>>operator in the List.Sort call.
>
>But if you don't use the '@', the compiler will try to execute the
>SortCompare function instead of passing it.
No it will not. Using the @ (address of) operator in this context only
achieves to remove the type checking of the parameter. Using the @
operator you can send in any function or procedure as the parameter
(including a local function as you used).
Removing the @ operator will restore the default type-checking in
Pascal. Trying to use a local function or the wrong number of
paramters will generate a compile-time error message.
Conclusions:
1. Use the correct procedure type for procedureal parameters (like the
one to TList.Sort).
2. Never use the @ operator in these cases. This will make the comiler
help you with point 1. The only exception is with routines that were
designed with local functions in mind (i.e. ForAll in Grids).
3. Work around bad call-back designs (like TList.Sort) that lack
cookie-values beeing sent to the call-back by using a global cookie
variable. If you need access to more than one value, make the cookie a
pointer to a local record in the calling routine.
For instance, in your case try something like (not tested):
type
PCookieRec = ^TCookieRec;
TCookieRec = record
Value: integer;
List: TList;
i: integer;
end;
var
Cookie: PCookieRec;
function SortCompare (Item1, Item2: Pointer): Integer;
var
j: integer;
begin
with Cookie^ do
begin
// Do your thing here.
j := Value;
Result := 0;
end;
end;
procedure TTest.DoIt (Value: integer);
var
CookieContents : TCookieRec;
begin
CookieContents.Value := Value;
with CookieContents do
begin
List := TList.Create;
try
List.Add (Pointer (1)); // Add a couple of items, so we have
List.Add (Pointer (2)); // something to sort
List.Sort (@SortCompare);
finally
List.Free;
end;
end;
end;
Chuck Jazdzewski
From: Hallvard Vassbotn <hall...@falcon.no>
>Even in VCL this technique is used, albeit only in the implementation
>part of the Grids unit (if you have the source code: search for
>TSparsePointerArray.ForAll).
Actually, it is also used in the MENUS unit in the menu merging code. We
didn't include it in Delphi because many people found it confusing in
Turbo Vision primarily because of the issues raised by Davor (in the
opposite sense).
In attempt to justify an arguably bad call, I decided not to include a
cookie parameter in the Sort method primarily for performance reasons. The
call itself was expensive and adding a push (at the time) to the stack
slowed it down significantly. In a QuickSort the compare is usually execute
much more than the swap. Also, I wanted to avoid local variables and stack
temporaries to get EBP used of P in my inner loops (adding a cookie would
have pushed the limit and generated a stack temporary). This did two
things, improved my inner loop and reduced stack consumption.
In Delphi 3, Sort was made more thread safe so looking at the current code
generation, adding an additional parameter is not significant, but I had to
make the call then. I planned for either the list to contain objects, where
the object could contain the cookie in a parent reference, or for there to
be multiple compare routines (e.g. CompareItemNames, CompareItemAmounts).
For good or bad, I choose speed over flexibility.
Chuck.