[fpc-devel] Debug compiler

[Ondrej Pokorny]

Thanks for the feedback!

> Independent of whether it will be integrated, there are definitely a 
> number things that should be changed first:
> * don't use the "is" operator unless there is absolutely no other way 
> to achieve the same effect. In the compiler, every node has a 
> "nodetype" field that can be used to determine the kind instead (which 
> is much faster than "is"
This is easily possible.

> * I'm not sure such a "virtual node" is the best approach. I don't 
> think we have that anywhere else in the compiler, so unless there is 
> no cleaner way, we shouldn't introduce this concept (if some nodes are 
> used differently from other nodes --other than the special error 
> node--, then maintenance and reasoning becomes harder)
> * don't add extra (semi-)global variables such as the 
> tscannerfile.inforin field unless there is absolutely no other way to 
> achieve the same effect. It's usually a quick hack, and there are way 
> too many of those in the compiler already (I've removed a number of 
> them over the years, but there are plenty left)
I don't know a way to achieve it without these 2 points. Maybe I'll 
think of something in the future. For now, they seem to be necessary :(
(But I don't know the compiler well, this was my first attempt to extend 
it, so I am not telling it is not possible. I just don't know any other 
way yet :)

Why I did it so:
inforin: the "enumerator" has to replace the property if and only if the 
evaluation is in the for-in statement. So you have to know that the 
property evaluation is done for the for-in-statement. If there is 
another way how to determine that the evaluation is done for the 
in-statement, it is possible to replace it (I don't know about it).
tenumeratornode "virtual node": it is definitely the easiest way (maybe 
not the best) because the enumerator evaluation is done for the parent 
object and not for the enumerator function. Thanks to tenumeratornode, 
you can use the same compiler code for:

*for x in MyObj do**
*and
*for x in MyObj.Objects do**
*(*Objects = property **Objects[Index: xyz]: zyx read GetObject 
enumerator GetObjectEnumerator;)

*-> these statements are semantically equivalent, with the difference 
that with "MyObj.Objects" you choose an alternative *GetObjectEnumerator 
*enumerator function instead of *GetEnumerator*. The enumerator function 
and parent object are handed over with tenumeratornode.

> * like for all other node types, create a "tenumeratornodeclass = 
> tclass of tenumeratornode" type, a "cenumeratornode: 
> tenumeratornodeclass = tenumerator;" global variable and always use 
> cenumeratornode.create() (so that the node class can be overridden by 
> an architecture-specific class if necessary)
This is easily possible.

> * it seems you have a memory leak (the tenumeratornode doesn't seem to 
> be freed anywhere)
No, there is no memory leak. tenumeratornode is destroyed in 'function 
for_in_loop_create(hloopvar: tnode): tnode;" in the last line: 
"expr.free;" - because expr is still the tenumeratornode - the expr 
parameter of create_for_in_loop is not var! You can easily check it by 
setting a breakpoint into tenumeratornode.destroy or run the compiler 
with heaptrc. There are some memoryleaks in ppc, but they were not 
introduced with the patch - there are the same memoryleaks with and 
without the patch.

Again, thanks for the feedback!

Ondrej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-devel/attachments/20151031/04b6e097/attachment.html>