Why does a record constructor misbehave in inline functions?

Question

In the following code the record constructor does something strange.
It works OK in all instances, except in the line marked below:

program Project9;

{$APPTYPE CONSOLE}
{$R *.res}

uses
  System.SysUtils,
  System.Generics.Collections;

type
  TIntegerPair = TPair<Integer, Integer>;

type
  TMiniStack<T> = record
  public
    Items: array[0..10] of T;
    SP: integer;
    procedure Init;
    procedure PushInline(const Item: T); inline;
    procedure PushNormal(const Item: T);
  end;

procedure TMiniStack<T>.Init;
begin
  FillChar(Items, SizeOf(Items), #0);
  SP:= 0;
end;

procedure TMiniStack<T>.PushInline(const Item: T);
begin
  Items[SP]:= Item;
  Inc(SP);
end;

procedure TMiniStack<T>.PushNormal(const Item: T);
begin
  Items[SP]:= Item;
  Inc(SP);
end;


procedure RecordConstructorFail;
var
  List1: TMiniStack<TIntegerPair>;
  List2: array[0..2] of TIntegerPair;
  Pair: TIntegerPair;
  a: string;
begin
  Writeln('start test...');
  FillChar(List1, SizeOf(List1), #0);
  List1.Init;
  List1.PushInline(TIntegerPair.Create(1, 1));
  List1.PushInline(Pair.Create(2, 2));   <<--- Failure
  List2[0]:= TIntegerPair.Create(1, 1);
  List2[1]:= Pair.Create(2, 2);
  if (List1.Items[0].Key <> 1) or (List1.Items[1].Key <> 2) then Writeln('something is wrong with List1-Inline');
  if (List2[0].Key <> 1) or (List2[1].Key <> 2) then Writeln('something is wrong with List1');
  List1.Init;
  List1.PushNormal(TIntegerPair.Create(1, 1));
  List1.PushNormal(Pair.Create(2, 2));
  if (List1.Items[0].Key <> 1) or (List1.Items[1].Key <> 2) then Writeln('something is wrong with List1-Normal');
  Writeln('Done');
  Readln(a);
  Writeln(a);
end;

begin
  RecordConstructorFail;
end.

Why does this line cause a failure?

List1.PushInline(Pair.Create(2, 2)); <<- Failure: Dumps the data somewhere else.

Is it a compiler bug?
Or am I missing something?

I'm using Delphi XE6.

`TIntegerPair.Create(1, 1)` is the correct syntax. However, record constructors are an abomination and Emba are crazy for pushing them on us. Class static functions are what should be used. Then no danger of confusion with constructors of classes. — David Heffernan, Aug 30 '14 at 08:37
What does "Failure" mean? Does it fail to compile? Does it raise an exception? Does it do something other than what you expected? — , Aug 30 '14 at 08:56
@DavidHeffernan, I fully agree, record constructors can't even be inlined. — Johan, Aug 30 '14 at 08:57
My advice is to ignore them. But yes it does look like a compiler bug. That said, I'm not sure what it means to call a record constructor on a variable. Would never think to do it. — David Heffernan, Aug 30 '14 at 08:59
Offending line caused undefined behavior; i.e. it dumps the data in some random location sometimes and other times it just corrupts the stack. — Johan, Aug 30 '14 at 09:00
The problem is that it's an easy mistake to make, because the line works OK with normal functions, but fails with inline functions. I've seen quite a lot of code that uses this construct (prob because it is short to type). — Johan, Aug 30 '14 at 09:02
To me this boils down to what the expression `Pair.Create(...)` evaluates to. The documentation appears silent on the subject. — David Heffernan, Aug 30 '14 at 09:23
ISTM that `Pair.Create(...)` is indeed the problem. I agree that it should not compile. And when it compiles, you get undefined behaviour. The only allowed syntax is, IMO, `TIntegerPair.Create(...)`. — Rudy Velthuis, Aug 30 '14 at 10:43
The reason it works with the non-inlined function is that the const parameter is passed as a pointer to tpair in a register. The register happens to have the correct pointer in it in the normal call. In the inlined call it does not. The syntax should not compile though. — Johan, Aug 30 '14 at 10:54
@RudyVelthuis `Pair.Create(...)` is legal syntax too but it does not return a value and cannot be used as an argument. The fact that compiler does not detect syntax error here is a compiler bug (IMO). — kludg, Aug 30 '14 at 13:57
@DavidHeffernan Why are record constructors "an abomination"? — David, Aug 30 '14 at 20:21
@DavidM Because when you read them, they look just like constructors for a class and so you assume that you need to call `Free` down the line. What's more, they don't construct, they initialise. Records are always constructor (created) some other way. Often automatically as locals or members of compound structures, or through calls to heap alloc. But never through record constructors. So, it is oxymoronic to call something a constructor is construction is one thing that it does not do. — David Heffernan, Aug 30 '14 at 20:39
@user246408: I know that it is valid syntax on an instance too, but the compiler should not allow its use as a parameter. That is what I meant. — Rudy Velthuis, Aug 31 '14 at 22:19

David Heffernan · Accepted Answer · 2014-08-30T11:48:26.750

In my view, inlined or not, an expression that is a constructor call on an instance does not evaluate to a value. And so cannot be passed as an argument. Think of

Pair.Create(...)

as though it is a procedure. It yields no value.

In my view the code should be rejected by the compiler and the fact that it is not rejected is the bug.

That the code appears to work when you don't inline it is, in my view, down to chance.

This is all guesswork though. Record constructors aren't properly documented. The documentation for constructors is for classes and has not been updated to cover records. The documentation for classes says that the instance form is an expression that is a reference to the instance.

When a constructor is called using an object reference (rather than a class reference), it does not create an object. Instead, the constructor operates on the specified object, executing only the statements in the constructor's implementation, and then returns a reference to the object.

But this doesn't make much sense for a record because there are value types rather than reference types.

My best guess is that the parser regards this expression as being the same type as the instance, because that's how it is for constructors of classes. But the codegen doesn't actually yield a value.

Actually the quoted text makes sense for records too. When called with record type prefix a record constructor acts as a class function and returns a value; when called with record instance prefix a record constructor acts as an instance procedure and does not return anything. — kludg, Aug 30 '14 at 13:44
@user except that the text says *and then returns a reference to the object* — David Heffernan, Aug 30 '14 at 13:47

Why does a record constructor misbehave in inline functions?

1 Answers1