[fpc-devel] Re: Regarding the Free Pascal closure branch

[Sven Barth]

On 03.03.2013 17:00, Blaise Thorn wrote:
> On 29.01.2013 13:21, Sven Barth wrote:
>
>> Am 29.01.2013 07:34, schrieb Blaise Thorn:
>>
>>> On 16.12.2012 20:52, Sven Barth wrote:
>>>
>>>> out of curiosity I have taken a look at your closure branch. Can it
>>>> be that you forgot to commit the pnameless unit?
>>>
>>> I did not actually forget, just got distracted,
>>
>> I know that problem... :)
>>
>>> and somewhat discouraged by the lack of readiness to accept small
>>> unrelated bugfixes directly.
>>
>> I've looked at the mails from that time again. Jonas has said that you
>> should do a bug report for things like that. Did you do that?  If not
>> then it's more or less clear why it got lost... one does not always
>> have the time to incorporate even a small fix, maybe because the
>> current checkout is polluted with other changes, etc. Thus it's indeed
>> normally the best to report such things in Mantis so it's not (as
>> easily) forgotten.
>
> 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.
>

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...

>> 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.

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...

>>> Shortly thereafter, a Delphi beta test had started, and I am usually
>>> very involved with those, to the point of having no free time at all.
>>>
>>> In the past weeks, I did try sync'ing my branch with the trunk, but
>>> only got up to r21067; there the JVM backend integration happened, so
>>> some careful manual merging is required, for which I have not yet
>>> found time.
>>>
>>> Let's say if I don't find it within a week, I'll just commit
>>> everything as-is and be done with it.
>>
>> As long as your branch compiles (and thus can be tested) someone else
>> can do the hard work of merging it to trunk.
>
> BTW, it's the things like GOTO (and occasional "WTF" comments) that I
> feel obligated to clean up before becoming "done with it".

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

>>>> if the main feature "closures" is already finished your work could
>>>> then already be reviewed for merging (I don't know the current state
>>>> of the branch, so I can't comment here).
>>>
>>> I expect it to work for the most but not all cases (for example,
>>> there should be some issues with capturing the parent variables from
>>> nested routines), but I have not actually written any comprehensive
>>> tests yet.
>>>
>> Did you write any tests and do you still have them? If so it would be
>> nice if you'd commit them as well.
>
> Those were pretty rudimentary. I am not sure if they can be qualified as
> tests :)

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