[fpc-devel] Debug compiler

[Ondrej Pokorny]

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

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