[fpc-devel] Closures/anonymous functions update

[Ryan Joseph]

After 2 months now I’ve not been able to get the required sources to help finish the closures branch so I guess it’s best to go to plan B and see if it’s feasible to finish at the last public repository I found. The author Blaise did manage to contact me once but then went silent so I don’t know what happened.

I’ve found a few bugs that need fixing but most of it seems to work as far as I’ve tested. Blaise claimed there were some "lifetime management issues” but I can’t find them yet (maybe this was an older version he was referring to). The only serious bug I can’t figure out yet is a runtime crash when declaring anonymous functions in class constructors.

Here’s what I managed to salvage so far with some requested changes made (see below).

https://github.com/genericptr/freepascal/tree/closures

Here’s the list of old comments from 2015-2016 which I was able to find using my detective skills and I corrected some of them. 

The only concerning thing is that Jonas had a request (reasonable however challenging) that the variable capturing process be unified with the existing code for nested procedures. I’m struggling to understand how this works however and I’m not certain if a) this is still important and b) if it’s even a good. The closures actually have a very minimal footprint in other parts of the compiler and is very non-invasive as it is. My feeling is that it’s not worth it to merge the two methods as it would complicate both but I’m really not sure. Anyways, if that’s required than this is the by far the biggest chunk of work left because it requires redoing a significant part of the design.

I wanted to make an alternate capture mode as an optimization for closures that don’t get passed outside of their scope but I wasn’t able to figure this out yet (using old-style objects instead of classes). It’s a waste to allocate a new class if not needed (this would hurt realtime graphics applications badly) so that should be tackled eventually. Maybe they can be made to inline or something, not sure how to handle it though.

What does the compiler team think about this? Can you guide me so I could finish what was started? Please update me on what needs to be done to get this moving along.

// https://fpc-devel.freepascal.narkive.com/ONaPiCka/closures-anonymous-methods
// http://lists.freepascal.org/pipermail/fpc-devel/2016-January/036479.html

{ 
  ===== Jonas:

  First of all, thanks for your work, and sorry for the late reaction.

  I was wondering whether you could rework the above functionality based  
  on the code in ncgnstld.pas/ncgnstmm.pas. That is code which is used  
  by the JVM and LLVM code generators to access nested variables. It  
  adds all variables that are used by nested routines to a record, and  
  then passes a pointer to this record as "parentfp" parameter (since  
  those platforms do not have a stack or frame pointer register that can  
  be passed on). This seems quite similar to what you're doing with  
  load_captured_sym() etc. Alternatively, maybe your code is better and  
  that code could be refactored to make use of your routines. I have not  
  yet studied your code in detail yet.

  One advantage would be that the existing code already handles this too  
  (for plain nested routines):

  ===== Sven:

  Sorry that it took me so long to answer. I have looked at your changes 
  shortly after you published them, but then I forgot... Mea culpa.

  First of thank you for the work you've done. I think I've said this in 
  the private mail I sent you already, but better I do it here as well. :)

  Now of course there are still quite some things aside from those that 
  you mentioned above. I'll just list them in no particular order:

  √ use our usual license header in pnameless.pas (see for example ogomf.pas)

  √ include the fpcdefs.inc in pnameless and don't use mode Delphi

  √ please don't use features (in this case C-style operators) that are 
  neither enabled by default in mode ObjFPC nor a enabled in fpcdefs.inc

  √ I'd prefer if you'd use the term "anonym" instead of "nameless" as 
  this way one not only can derive more easily that you mean anonymous 
  functions, but in addition to that the unit name would fit into the 8.3 
  scheme (this point includes both the unit name as well as the 
  oo_is_nameless and po_nameless flags)

  √ don't have defcmp depend on pnameless; instead move 
  are_compatible_interfaces() and are_compatible_headers() to defcmp (you 
  don't need to put everything and the kitchen sink into the pnameless unit)

  - analogous for load_contextual_self(); maybe even move that to a new 
  unit altogether as this doesn't really deal with nameless/anonym 
  functions, but more with the closure part of the concept (so either 
  nutils.pas or a new nclosure.pas as it's dealing more with nodes than 
  parsing)

  √ do *not* call internalerror if the user can easily trigger it; in that 
  case it would be the one in factor() of pexpr.pas

  √ use a "anonymousfunctions" modeswitch instead of checking for mode 
  Delphi; there might be users who want to use them in other modes as well

  √ the flag oo_is_nameless (or oo_is_anonym) is okay; we won't go for 
  type coersion

    - get rid of parse_method_reference(); instead integrate the code into 
    procvar_dec() and have that return the interface def based on a boolean 
    flag (Note: your code as is would result in wrong parsing if modeswitch 
    blocks would be activated in mode Delphi)

    - based on the above point you can also get rid of the 
    ppm_method_reference flag; the remaining ppm_nameless_routine I'd 
    instead convert into a set together with the isgeneric parameter

  Those are my observations so far. I'm still a bit uneasy with the 
  capturing process itself though. Maybe Jonas' remark will help there to 
  clean things up a bit and to unify the two approaches a bit (I know that 
  they can't be completely unified as one uses records while the other 
  uses interfaces and a backing storage class). I'd also like to keep the 
  possibility in mind for alternate capturing modes (e.g. copy instead of 
  reference); of course this would require additional syntax, but if the 
  backend already supports it then this would at least allow such an 
  extension easily.

  Also important are tests. And I don't mean those that you added as dpr 
  files, but tests that can be run in our testsuite, tests that need to 
  successfully run, merely successfully compile or even need to fail to 
  compile (look at the files in tests/test/ to see what I mean).
}

Regards,
	Ryan Joseph