<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">I have thought about the problem and I
have some new ideas:<br>
<br>
On 31.10.2015 12:06, Jonas Maebe wrote:<br>
</div>
<blockquote cite="mid:5634A0C7.3060503@elis.ugent.be" type="cite">Independent
of whether it will be integrated, there are definitely a number
things that should be changed first:
<br>
* 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"
<br>
</blockquote>
<br>
I did it and uploaded a new patch to the issue report.<br>
<br>
<blockquote cite="mid:5634A0C7.3060503@elis.ugent.be" type="cite">*
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)<br>
</blockquote>
<br>
The "virtual node" in the comment may be misleading. I changed the
comment. I now think that actually the use of tenumeratornode is a
pretty clean way to do it - it completely reflects the code usage.
If I comment on the attached project1.lpr from the issue report:<br>
<br>
The use of array property enumerator:<br>
<b> for s in Self.StringArray do</b><b><br>
</b><b> Writeln(s);</b><b><br>
</b><br>
is the same as using the default enumerator (only with a different
enumeration fuction):<br>
<b> for s in Self do</b><b><br>
</b><b>
Writeln(s);</b><b><br>
</b><b>
</b><br>
It means that internally you have to create the for-in loop with
"create_enumerator_for_in_loop" with the same "expr" argument
(pointing to Self) and with "pd" pointing to the actual enumeration
function. tenumeratornode creates the link between
"create_for_in_loop" and "handle_propertysym", which again reflects
the syntax pretty well.<br>
<br>
The argument that the tenumeratornode concept is completely
different from other nodes is correct - but if you think about the
syntax you will see that also the syntax is completely different
from other pascal syntax so you need such a unique node. In the
for-in case for array properties, the property is not evaluated as
usual but the appropriate enumeration function and object have to be
passed over to the for-in loop. tenumeratornode doesn't have any
other meaning and cannot/must not be used anywhere else - so there
shouldn't be any maintenance problems either.<br>
<br>
<blockquote cite="mid:5634A0C7.3060503@elis.ugent.be" type="cite">*
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)<br>
</blockquote>
<br>
If you know any other way how to detect that the compiler is
evaluating the for-in section in the "handle_propertysym" procedure,
I'll be happy to change it! I myself don't know it :(<br>
<br>
<blockquote cite="mid:5634A0C7.3060503@elis.ugent.be" type="cite">*
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)<br>
</blockquote>
<br>
Actually since tenumeratornode is pretty special and the only things
used from tenumeratornode are the public variables <b>enumowner</b>,
<b>enumproc</b> and <b>resultdef</b> - there is nothing to
override. If you think the override concept is needed (I don't think
so) then <b>enumowner</b>, <b>enumproc</b> should be moved to
protected section and virtual getters <b>get</b><b>enumowner</b>
and <b>getenumproc</b> should be introduced and used in
"create_for_in_loop". I can do it but I don't see any usage for it.
(I may be wrong here, so if you tell me I should do it, I'll do it.)<br>
<br>
<blockquote cite="mid:5634A0C7.3060503@elis.ugent.be" type="cite">*
it seems you have a memory leak (the tenumeratornode doesn't seem
to be freed anywhere)<br>
</blockquote>
<br>
As I commented in my previous answer - there is no memory leak.<br>
<br>
Again, thanks for the feedback!<br>
<br>
Ondrej<br>
</body>
</html>