I have thought about the problem and I have some new ideas:
On 31.10.2015 12:06, Jonas Maebe wrote:
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"
I did it and uploaded a new patch to the issue report.
* 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)
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:
The use of array property enumerator:
* for s in Self.StringArray do**
** Writeln(s);**
*
is the same as using the default enumerator (only with a different
enumeration fuction):
* for s in Self do**
** Writeln(s);**
***
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.
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.
* 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)
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 :(
* 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)
Actually since tenumeratornode is pretty special and the only things
used from tenumeratornode are the public variables *enumowner*,
*enumproc* and *resultdef* - there is nothing to override. If you think
the override concept is needed (I don't think so) then *enumowner*,
*enumproc* should be moved to protected section and virtual getters
*get**enumowner* and *getenumproc* 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.)
* it seems you have a memory leak (the tenumeratornode doesn't seem to
be freed anywhere)
As I commented in my previous answer - there is no memory leak.
Again, thanks for the feedback!
Ondrej
_______________________________________________
fpc-devel maillist - [email protected]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
