[fpc-devel] Closures

[Sven Barth]

On 09.12.2015 11:43, Blaise at blaise.ru wrote:
> On 09.03.2015 16:36, Blaise at blaise.ru wrote:
>> On 15.01.2012 18:26, Blaise wrote:
>>> I have implemented the support for the Delphi-compatible non-generic
>>> closures.
>> I am ready to commit the improved (and fully compilable) version.
>
> Four years later, here comes (vastly improved) second attempt at a
> public release: http://hg.blaise.ru/public/fpc .
>
> My plan is to:
> 1) Collect the initial remarks and fix the most awful bugs and
> shortcomings that would prevent merging this into the trunk (I am using
> Hg for this step because I want to be able to seamlessly synchronise
> with trunk and branch at ease.)
> 2) When the overall design and implementation are deemed acceptable,
> correct the remaining coding style issues and push into my branch on the
> official SVN server.
> 3) Create a ticket and wait for the merge.
>
> Initial tests: http://hg.blaise.ru/public/fpc-tests .
>
> A) Things that work (at least, expected to -- before the merge):
>      * embedding nameless routines into expressions;
>      * accessing outer variables/parameters/Selves from such routines
> and from routines nested in them;
>      * new method reference type;
>      * structural type equivalence for method references;
>      * 100% Delphi-compatible design and implementation, including two
> crucial points:
>          1) closure objects are descendants of TInterfacedObject;
>          2) method references are half-hidden interface types.
> B) Things that do not work (not planned to be addressed before the merge):
>      * accessing outer local-variables/parameters/Selves that are
> located more than one level higher than the nameless routine;
>      * omitting currently mandatory () for invoking method references
> without arguments;
>      * assigning normal routines and methods to method references;
>      * support for generics.
> C) Future possibilities:
>      * special handling for nameless routines that do not capture anything.
> May go into A) or B): capturing large parameters by value may have
> issues -- needs checking.
> Known bugs, mostly for A):
> http://hg.blaise.ru/public/fpc-tests/file/tip/closures/TODO .
>

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)
- adhere to the coding style of the compiler; this includes quite some 
code in pnameless.pas (e.g. " := " or variable declarations with ": " 
instead of " : "), but more importantly to pexpr.pas for example where 
you used "ELSE AGAIN:=FALSE" which should be
"else
   again:=false"

instead
- 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,
Sven