On 03.11.2015 11:55, Sven Barth wrote:
>> - extend comp_expr by another boolean parameter (which is set in
for_in_loop_create) and pass that down to factor (even better: convert
the boolean parameters of comp_expr, sub_expr and factor to a set,
would be cleaner anyway, IMHO) (pro: the state is only maintained
locally and new flags can be added easily; con: a greater change in
the compiler, though that would be a onetime thing)
>
>
> Looks unnecessarily complicated for me as well.
And here we disagree. The "inforin" you added to tscannerfile is
something we don't want to see introduced (especially since we're
trying to reduce reliance on such (pseudo)global state) and the only
way to fix this is by passing down the flag through comp_expr to
handle_propertysym.
Oh, here you are talking about the inforin global state. No, we don't
disagree here. I am absolutely OK with replacing the inforin for a
parameter in "handle_propertysym". But this doesn't have anything in
common with the use of tenumeratornode. You can replace inforin with a
different approach but it still doesn't mean that you automatically
solved the problem with the use of tenumeratornode and you don't need it.
Note that you have to handle the enumerator property keyword in
*handle_propertysym*. You have to return the enumerator function from
palt_enumerator list here. (Issue 1)
The result then has to be correctly interpreted in *create_for_in_loop*.
(Issue 2)
If we want to solve Issue 1 in a clean way (that means support property
enumerator only in for-in), we need either a global state (inforin) or a
new parameter for *handle_propertysym* and all the functions on top of
it. I am absolutely OK with both variants.
If we want to solve Issue 2, we either need a new node (tenumeratornode)
or a new varible/flag in tcallnode. It is not sufficient to know that we
are in a for-in loop. We have to know that the returned node from
*create_for_in_loop* really is the enumerator to use.
> From my POV, having thought about the problem for some days already,
the use of tenumeratornode is the simplest and clearest way to achieve
the goal. The property array enumerator use in for-in is a "single
case" only. So the use of a single-purpose node is clear as well -
everybody understands on the first sight what is going on.
No, I think using a tcallnode is a more powerful approach.
create_for_in_loop only needs to check that the tcallnode returns a
valid enumerator type (namely one containing "Current" and "MoveNext")
and be done with it. This way one can easily extend the for-in with
other functionality if the need arises.
This is not save. This approach could interfere with current code. What
about if your object contains "Current" and "MoveNext" but is not the
actual enumerator to use? If you use for-in on a
variable/function/whatever that returns such an object, the object
"Self" will be used as the enumerator and not the "Self.GetEnumerator".
As a result you get wrong enumerator object and, furthermore, your
object gets destroyed after the for-in loop because enumerator objects
are automatically freed. I can write you a sample code to prove it.
=> Don't do it :)
>
> It is way more dangerous and unclear if you use some flags or
parameters that change the tcallnode handling. If you use
tenumeratornode you clearly see in what code parts it is used and how.
If you use tcallnode + some flag/parameter it will be devil. For such
reasons there was OOP invented, so we should take advantage of it.
I'm not talking about the handling of tcallnode I'm talking about when
handle_propertysym is returning the enumerator or not (completely
independent of the usage of tenumeratornode or tcallnode).
> If you don't want to include the "enumeratorn" into tnodetype and
the "is" operator from my first proposal is too slow, you can also
directly type check for tenumeratornode in create_for_in_loop (in the
case you don't allow tenumeratornode ancestors, of course):
This has nothing to do with the discussion we're currently having. If
a new node would be introduced it would have to be implemented exactly
like it's done with the other nodes, namely using nodetype and
potentially being extendable. There are reasons for this approach
after all.
I am completely for this! It was just an idea if you /should//not/ want
to introduce a new node. If you are not principally against having a
complete new node, forget the idea :)
Ondrej
_______________________________________________
fpc-devel maillist - [email protected]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
