Hello.

I have implemented the support for the Delphi-compatible non-generic closures.

Gradually, I plan to extend that support with:

  * generic closures (I am not sure, though, how much current generics are expected to be compatible with Delphi);
  * Delphi-incompatible extensions (simple nameless routines, without capturing of variables, for one).

There are also several Delphi-compatibility fixes that should be made, not directly related to closures.

My question is: how the work should be organised? Should a separate SVN branch be created for this?

Blaise Thorn wrote

My question is: how the work should be organised? Should a separate SVN branch be created for this?

The best is to create an svn branch. See private mail.

‎The best way is to use distributed version control imho. Git, hg etc provide the ability to store the changes locally and push them also to remote places.
It also allow me not to disturb you when working on the same files for different needs.

My 2 cents in the matter (Graeme behind you :))

Yes, and probably pushed somewhere nobody notices or lost with the next hd crash because never pushed. Having it in the central fpc svn repository allows everybody to review it what e.g. I try to do with every commit to the central fpc svn repository.

It also allow me not to disturb you when working on the same files for different needs.

The same applies for a svn branch.

Well... at least for me the use of Git (together with Git-SVN) provided the possibility for me to commit code while on the train (I have a data flatrate only since August and even then doing a SVN commit or update while on the train is sometimes not the best ^^) [also I didn't want to sync every night when at home, because often I'd simply forget that ;) ]

Regards,
Sven

In our previous episode, Sven Barth said:

I use SVN on the train too. If you need a checkout, just copy the repo and revert.

Maybe you're traveling on a track where there aren't loses of signal ;)

Regards,
Sven

"Blaise Thorn" wrote

Hello. I have implemented the support for the Delphi-compatible non-generic

closures.

Gradually, I plan to extend that support with: * generic closures (I am not sure, though, how much current generics are

expected to be compatible with Delphi);

How firm are you with the implementation of generics inside the compiler? If you're not that firm it might be better if I implement generic procedures/methods first and then you can build on that (as that should hopefully be reusable by you).

Regards,
Sven

Sven Barth wrote

How firm are you with the implementation of generics inside the compiler?

Totally blank. Since you are the person who has been working on that area most recently, I would actually appreciate if you could compile a document (on the wiki?) outlining the internal workings of generics in FPC; for the current documentation, I understand, is quite lacking. On my part, I already have a document (only in Russian, though) describing the implementation of closures; I will translate and publish it after my initial commits (which I expect to be on We/Th this week).

If you're not that firm it might be better if I implement generic procedures/methods first and then you can build on that (as that should hopefully be reusable by you).

For now, I don't reckon that any ordering is required. Note that the only thing in closures that can have type parameters is method references

	type M<T> = reference to procedure(const X: T);

which are converted, internally, into generic interfaces. A nameless routine (closure) can only /refer/ to generic type parameters of the enclosing routine/class, and I hope that making it generic-aware/generic-safe wouldn't be that hard.

Might be an idea... currently I'm a bit busy with university, but after that (beginning or middle of February) I hope to fully start through again :)

Ahhh, now I see. I haven't used closures yet, so I didn't know exactly in what context generics can be used. Depending on how you parse the "reference to" (are you using the parsing of procedure variables here?) then you might only need to add the type such a closure is parsed as to the types for which generics are supported. If they are indeed converted to interfaces you might not have to change that much at all...

Btw: in mode ObjFPC (or non-Delphi modes in general) the declaration would then look like this (to be conform with other generic declarations):

type
   generic M<T> = reference to procedure(const X: T);

Again you shouldn't need to change that much.

Regards,
Sven

Blaise Thorn wrote

I have implemented the support for the Delphi-compatible non-generic closures.

OK, I have started committing into http://svn.freepascal.org/svn/fpc/branches/blaise/closures (Be warned, though: I have not even finished with the work I have already done => the branch is not yet compilable.)

For know, I am here for a different request: I reckon that small fixes, especially those not directly related to closures, ought to be submitted separately. I understand I have no write access to the trunk, so I am attaching the patch here.

Changes for consume_generic_type_parameter():
! memory leak (TFPHashList) for every non-generic routine ! type_e_generic_declaration_does_not_match was never emitted

Blaise Thorn wrote

For know, I am here for a different request: I reckon that small fixes, especially those not directly related to closures, ought to be submitted separately.

Or I could create something like http://svn.freepascal.org/svn/fpc/branches/blaise/misc and ask for its periodical reintegration into the trunk. The only tricky part with SVN, as I understand, is to "reopen" such branch after every reintegration.

The easiest way for us would be if you could file a bug report for every independent patch (with a test program). Your proposed misc branch is also possible, but in that case make sure to also add test programs when fixing things. Re-syncing it with trunk after merging should be no problem as long as nobody changes the indentation of your patches or so.

Jonas

You know, I am all for a formal process -- in fact, I did use to file bug reports even for tiny patches when they concerned the user-visible changes -- but filing bug reports for patching a memory leak on line XXXX of YYYY.pas is definitely an overkill in my book.

Nevertheless I've now looked at your patch regarding pdecsub.pas. The problem with the not printed message is already fixed in trunk and I've now incorporated your fix regarding the memory leak and will commit it once I've checked whether it breaks something.

Thank you. Actually, that was another thing that discouraged me. I intended to ensure that my branch did not introduce any memory leaks, but then I found out that FPC leaked like a , especially when the error recovery was concerned.

I also remembered other mails regarding your progress (where you e.g. "complained" that you used "goto"), but I didn't find them anymore :(

You are mistaken. I did not discuss anything past the few initial messages.

BTW, it's the things like GOTO (and occasional "WTF" comments) that I feel obligated to clean up before becoming "done with it".

Those were pretty rudimentary. I am not sure if they can be qualified as tests :)

Anyway, I have started the merging process and, as I have seen the today's thread in the dev's list, I am CC'ing this message there.

But on the other hand you see that they can be easily forgotten. E.g. if I forget to mark an e-mail as important it could be that I don't remember it for a long time or at all...

We try to fix memory leaks in the error case, because while the compiler will stop afterwards the text mode IDE doesn't and that one contains the compiler compiled in. So if you find any memory leaks don't hesitate to report them (you can also collect some and report them as one issue).

> I also remembered other mails regarding your progress (where you e.g. > "complained" that you used "goto"), but I didn't find them anymore :( You are mistaken. I did not discuss anything past the few initial messages.

But I remember that I've already read about your goto-point below... could have also been inside your code though...

Well... we aren't *that* strict about gotos in the compiler. Sometimes they make the code more readable than strange loops :)

One can always discard them later on, so I would add them. :)

Anyway, I have started the merging process and, as I have seen the today's thread in the dev's list, I am CC'ing this message there.

Thank you. I hope that Visiliy will continue your work and maybe your code will show him valuable information what he can/should do in the compiler.

Regards,
Sven

Blaise Thorn wrote

I have implemented the support for the Delphi-compatible non-generic closures.

I am ready to commit the improved (and fully compilable) version. Per Jonas Maebe's suggestion, I am going to create a new branch (also, last time I tried to commit into the existing "closures" branch, I was consistently getting 500 Internal Server Error).

I would like to acknowledge the participation of Craig Peterson of Scooter Software, who was persistent enough to nudge me into completing this, provided additional testing, and offered a compensation for the time spent on the last stage.

Whilst I mutilate the source code into compatibility with the FPC coding "style", I have some questions that can be raised even before the code is committed.

FPC trunk r30150, compiled with EXTDEBUG, emits a debug warning for the following program: ----------8<----------
type T = interface

	procedure Bar;

end;

function Foo: T;
begin
	result := nil
end;
begin
	Foo().Bar()
	// ^-- Warning: tgobj: (FreeTemp) temp at pos -44 is already free !
end.
----------8<----------
Does this indicate a problem in the compiler, or is this warning bogus?

Am 09.03.2015 um 14:36 schrieb

I'd assume that the warning refers to "result := nil" in Foo(). Assign
something different and try again to find out more.

DoDi

Hans-Peter Diettrich wrote

I'd assume that the warning refers to "result := nil" in Foo().

Wrong. And the comment clearly indicates to which line it refers. This happens because of the call chaining.

Do you already have an svn account?

It indicates a problem in the compiler. As the message indicates, it means that you (or someone else before you) are trying to free a temp (allocated via tg.gettemp or the like) twice. The generated code will still be ok in this case, but it could lead to trouble in case a temp would be allocated again in between which would happen to get the same address as the old one, since then the second free of the old temp will free the new temp.

Jonas

Jonas Maebe wrote

Do you already have an svn account?

The one I had back in 2012.
My old branch is here: http://svn.freepascal.org/svn/fpc/branches/blaise/closures

It indicates a problem in the compiler.

Thanks.
Shall I file it in the bug tracker, then?

As the message indicates, it means that you (or someone else before you) are trying to free a temp (allocated via tg.gettemp or the like) twice.

I am slightly confused by your choice of the pronoun, so allow me to reiterate: the issue exists in the "stock" compiler, even without my patches applied.

The generated code will still be ok in this case, but it could lead to trouble in case a temp would be allocated again in between which would happen to get the same address as the old one, since then the second free of the old temp will free the new temp.

I am not at all familiar with this part of the compiler, so devising a test case, that would result in invalid codegen, would be difficult for me. Based on your explanation, it seems it would be something like Foo().Bar( YieldAnotherTemp() )
assuming that YieldAnotherTemp() is called after Foo()?

You should still have access to everything under http://svn.freepascal.org/svn/fpc/branches/blaise

> It indicates a problem in the compiler. Thanks. Shall I file it in the bug tracker, then?

Can't hurt.

Then it will be "someone else before you".

It depends on when exactly the second free of the initial temp occurs. A testcase that results in invalid code is not necessary, however. It should be easily debuggable without that (although fixing it may be more tricky).

Jonas

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 .

Great !

I am using a kind of home-brew pseudo closure (a function that ends with "free;") in a project I am preparing.I'd be happy to use the formally correct stuff.

-Michael

http://hg.blaise.ru/public/fpc

Sorry, forgot:
In case someone wants to play, but does not want to clone & compile for himself (and runs untrusted binaries), the executable for Windows/x86 is http://blaise.ru/fpc/fpc-closures-9.7z . Should work with compiled units from ftp://ftp.freepascal.org/pub/fpc/snapshot/trunk/i386-win32/fpc-3.1.1.i386-win32.zip . NB: built with EXTDEBUG -- beware of bogus diagnostic compiler messages.

2015-12-09 11:43 GMT+01:00 :

Sounds good. I'm waiting for this for a long time. Anonymous methods are cool :)

Blaise wrote on Wed, 09 Dec 2015:

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):

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;

Jonas

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