Anonymous method delegate does not capture

Issue #7 new
Anders Melander created an issue

When using an anonymous method as a delegate the compiler should either capture referenced external variables or disallow references to them.

Currently the compiler treats anonymous delegates as closures but doesn't perform capture.

The following script reproduces the problem.

type
  TEvent = procedure;
type
  TMyClass = class
  private
    FEvent: TEvent;
  public
    constructor Create;
    procedure DoIt;
    property Event: TEvent read FEvent write FEvent;
  end;

procedure TMyClass.DoIt;
begin
  if (Assigned(FEvent)) then
    FEvent;
end;

constructor TMyClass.Create;
begin
  var x := 'hello';
  FEvent := procedure
    begin
      ShowMessage(x); // Displays blank string
      ShowMessage(ClassName); // Causes "Range check error" in TVarParentExpr.EvalAsVariant
    end;
end;

begin
  var MyClass = TMyClass.Create;
  MyClass.DoIt;
end;

Comments (5)

  1. Eric Grange repo owner

    Closures are currently only parsed, and only meant for codegen purposes (under Smart)

    The script engine itself does not support closures, much work (and many extra tests) would be required to support them.

  2. Eric Grange repo owner

    Anonymous methods (as in Delphi) are closures. You should be getting an error at compilation on the "FEvent := procedure" line (I am getting one when compiling your snippet)

  3. Eric Grange repo owner

    coAllowClosure is only to be used for CodeGen (when the compiler is used to generate an AST), it is not supported for script engine execution.

    (the one exception being references to global variables (issue #3)).

    They will not support local variables properly either, nor parameters, and they generally will not setup the stack correctly.

    To support them the current stack-based engine would need to be changed to a closure-based engine, this would have other benefits in the long run, in terms of reducing the dependence on variants. The other implementation option would be to handle them through implicit anonymous classes & interfaces, like Delphi does, but this would only add complexity with no long term side benefits.

  4. Eric Grange repo owner

    It could work only if the implementation does not reference anything in its parent scope, ie. that it is a top-level static function declared locally, and not a method or a delegate. However in that case, this is a very limited usage case, and not an anonymous "method" as usually understood (and used), and there are not enough unit tests at the moment to check if all the issues above would be properly detected.

    In practice, any reference to something stack-related (variable, parameter, field, method...) in the parent scope will have been parsed as a stack-relative reference (as for a regular local function), but if you acquire a pointer to that local function, it means it can be invoked after the parent scope has ended, or from code the parent scope will have called, and in both cases the stack-relative reference will be incorrect.

    In your initial snippet "ShowMessage(x);" is compiled as for a local function, so as something like "ShowMessage(VariableAtStackOffset(-3))", the -3 is only meaningful if the function is code directly from within the context of TMyClass.Create, it's not valid once this constructor has exited, or or even from any method this constructor would have called (as the -3 would then point to something in the stack that belongs to the method the constructor called).

    I also suspect that merely strengthening the code and test suite to detect all the potential issues would be of similar complexity to the initial stages of closure support, but unlike closure support, would be wasted work once work on closures begins.

  5. Log in to comment