[fpc-devel] Debug compiler

[Sven Barth]

> On 02.11.2015 20:24, Sven Barth wrote:
>>>
>>> The only thing we need is a new flag/whatever so that
>>> *create_for_in_loop* knows that "tcallnode(expr)" contains an
enumerator
>>> function. tnode.flags seems to be full :( Do you have any suggestions?
>>> If we sort this out, 2 more arguments against it are nil :)
>>
>>
>> I see three possibilities to avoid the addition of a global flag:
>> - simply always return the enumerator function if no arguments are given
for an indexed array (pro: easy to implement; con: will bite us once we add
another functionality that works on the array property as a whole)
>
>
> Yes, this is not very clean. Furthermore such code would compile as well:
>
> x := MyObj.MyArrayProperty;
>
> (x would get the enumerator function, which is wrong.)

Not if we define it as the intended purpose. But also note that I had
mentioned in my "con" that defining it like this might bite us layer on if
we have a different usecase for code like the above. So I don't favor that
option either anyway.

>
>> - always return the enumerator, but add checks everywhere except the
for-in parsing against the enumerator (maybe for this case the enumerator
node would be an advantage) (pro: enumerator will only work in for-in; con:
every expression handling code needs to maintain that code)
>
>
> Looks unnecessarily complicated.

That's my opinion as well ;)

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

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

>
> 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.
However I'm still not of the opinion that the enumerator node is needed at
all.

Regards,
Sven
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-devel/attachments/20151103/953b6567/attachment.html>