You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Michael Schmalle <ap...@teotigraphix.com> on 2013/03/06 16:22:14 UTC

[FalconJx] binary operator parenthesis

Erik,

Now that I'm getting back into the core framework again I am wishing  
that I would have taken care of a bug that I knew about.

There is a serious problem that has to be fixed. Since we don't save  
tokens from the parser, a Binary AST tree knows nothing about it's  
order of operations when output using the visitor framework.

The fortunate thing here is that when the ASParser assembles these  
nodes, they are assembled based on operator precedence.

The unfortunate thing is, when I go to fix this its more than likely  
going to affect a huge amount of tests.

My plan is to create tests based on my other project so I stay out of  
the main framework for now. When I figure out something that works I  
will let you know because we should again be on the same revision so  
there aren't hard merge issues.

Mike

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Michael Schmalle <ap...@teotigraphix.com>.
Quoting Erik de Bruin <er...@ixsoftware.nl>:

> Agreed (on the 'simple' fix).
>
> A thought: isn't doing this in Jx not somewhat over the top? I mean,
> if you are going to 'calculate' the parenthesis somehow, you might be
> fixing the user's code - after all, the user can put them in however
> he likes... Isn't there a way for the parenthesis to be (and this is
> probably going to sound silly, but I'm not so deep into this stuff
> that I know the actual jargon) part of the AST, so we can just 'visit'
> and 'emit' them?

Answering this question... NO, that is the problem, most parsers have  
a token stream that is accessible. At the time we have the AST trees,  
all tokens are gone. The binary trees are correct but have no parent  
"container" like parens have on say ParameterNodes or FunctionCallNodes.

Does that make sense?

If they created containers for all binary expressions, the trees would  
take up a huge amount of memory.

Mike




> EdB
>
>
>
> On Wed, Mar 6, 2013 at 4:37 PM, Michael Schmalle
> <ap...@teotigraphix.com> wrote:
>> Don't worry about this one.
>>
>> When I fix this, I'm just going to have to get every test working again
>> before I commit. I have time since this is a breaking bug that will render
>> complex binary expressions useless as it stands.
>>
>> What I plan on doing is the most simple fix. Since I know that the tree is
>> correct, I will probably implement this in the before and after strategy to
>> wrap any binary expression that is an operator in ( ). This might cause some
>> extra parenthesis that "might" not be needed but for now I see this as the
>> safest and most accurate way to proceed.
>>
>> Mike
>>
>>
>> Quoting Erik de Bruin <er...@ixsoftware.nl>:
>>
>>> Mike,
>>>
>>> Sure, no problem. I'm not planning any work on Jx the next couple of
>>> days, so if you figure something in that time, go ahead and implement.
>>> Also, if you need help fixing tests, I don't mind, so feel free to
>>> ask.
>>>
>>> EdB
>>>
>>>
>>>
>>> On Wed, Mar 6, 2013 at 4:22 PM, Michael Schmalle
>>> <ap...@teotigraphix.com> wrote:
>>>>
>>>>
>>>> Erik,
>>>>
>>>> Now that I'm getting back into the core framework again I am wishing that
>>>> I
>>>> would have taken care of a bug that I knew about.
>>>>
>>>> There is a serious problem that has to be fixed. Since we don't save
>>>> tokens
>>>> from the parser, a Binary AST tree knows nothing about it's order of
>>>> operations when output using the visitor framework.
>>>>
>>>> The fortunate thing here is that when the ASParser assembles these nodes,
>>>> they are assembled based on operator precedence.
>>>>
>>>> The unfortunate thing is, when I go to fix this its more than likely
>>>> going
>>>> to affect a huge amount of tests.
>>>>
>>>> My plan is to create tests based on my other project so I stay out of the
>>>> main framework for now. When I figure out something that works I will let
>>>> you know because we should again be on the same revision so there aren't
>>>> hard merge issues.
>>>>
>>>> Mike
>>>>
>>>> --
>>>> Michael Schmalle - Teoti Graphix, LLC
>>>> http://www.teotigraphix.com
>>>> http://blog.teotigraphix.com
>>>>
>>>
>>>
>>>
>>> --
>>> Ix Multimedia Software
>>>
>>> Jan Luykenstraat 27
>>> 3521 VB Utrecht
>>>
>>> T. 06-51952295
>>> I. www.ixsoftware.nl
>>>
>>
>> --
>> Michael Schmalle - Teoti Graphix, LLC
>> http://www.teotigraphix.com
>> http://blog.teotigraphix.com
>>
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Michael Schmalle <ap...@teotigraphix.com>.
I'm going to commit my changes;

I ignored 3 unit tests that I will get back to. I am not decided on  
how String literals are handled when a member field resolves to a  
String type. I have normal string concatenation not being parened IE  
"foo" + "bar" + "baz". I know how to fix it, just going to give my  
head time to think about it.

But right now "foo" + bar + "baz"

is getting ("foo" + bar) + "baz", I will fix this when I get the head.  
Other than that all tests pass.

Mike


Quoting Michael Schmalle <ap...@teotigraphix.com>:

> Heh,
>
> I wouldn't say awesome but I'm beginning to like this visitor  
> framework more every day because this fix wasn't intrusive at all.
>
> This proves we have it working right; Here is a test that is just plain weird
>
> a = (a + b) - c + d * e;
>
> It now produces;
>
> a = (((a + b) - c) + (d * e))
>
> Notice how it automatically gets the multiply right and deals with  
> the addition paren first.
>
> I might try to work on that outer paren in an assignment expression  
> but we could just leave it as is and it would be fine, just adds a  
> bit more k to a generated file. :)
>
> THere might be some edge cases but I will worry about those later.
>
> Mike
>
>
>
> Quoting Erik de Bruin <er...@ixsoftware.nl>:
>
>>> You're not think 5th dimensionally here;
>>
>> I rarely venture in such an integer state of mind. I'm more of a
>> fractal kinda guy :-)
>>
>>> Currently without fixing this bug a use has the expression statement of;
>>>
>>> var a = ((a + b) - (c + d)) * e;
>>>
>>> They use the compiler, it checks out fine, FalconJx is currently rendering
>>> to JavaScript that will get executed;
>>>
>>> var a = a + b - c + d * e;
>>>
>>> See a problem here?
>>
>> Yes, that's why I wrote you that TODO in the test ;-)
>>
>>> I have tested a fix that produces;
>>>
>>> var a:* = ((a + b) - (c + d)) * e;
>>
>> You are awesome! I might have to come visit you in your universe
>> sometimes, I'm sure you over-hyper-cube-landers can teach me a trick
>> or two.
>>
>> Looking forward to you commit,
>>
>> Have fun!
>>
>> EdB
>>
>>
>>
>> --
>> Ix Multimedia Software
>>
>> Jan Luykenstraat 27
>> 3521 VB Utrecht
>>
>> T. 06-51952295
>> I. www.ixsoftware.nl
>>
>
> -- 
> Michael Schmalle - Teoti Graphix, LLC
> http://www.teotigraphix.com
> http://blog.teotigraphix.com
>
>

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Michael Schmalle <ap...@teotigraphix.com>.
Heh,

I wouldn't say awesome but I'm beginning to like this visitor  
framework more every day because this fix wasn't intrusive at all.

This proves we have it working right; Here is a test that is just plain weird

a = (a + b) - c + d * e;

It now produces;

a = (((a + b) - c) + (d * e))

Notice how it automatically gets the multiply right and deals with the  
addition paren first.

I might try to work on that outer paren in an assignment expression  
but we could just leave it as is and it would be fine, just adds a bit  
more k to a generated file. :)

THere might be some edge cases but I will worry about those later.

Mike



Quoting Erik de Bruin <er...@ixsoftware.nl>:

>> You're not think 5th dimensionally here;
>
> I rarely venture in such an integer state of mind. I'm more of a
> fractal kinda guy :-)
>
>> Currently without fixing this bug a use has the expression statement of;
>>
>> var a = ((a + b) - (c + d)) * e;
>>
>> They use the compiler, it checks out fine, FalconJx is currently rendering
>> to JavaScript that will get executed;
>>
>> var a = a + b - c + d * e;
>>
>> See a problem here?
>
> Yes, that's why I wrote you that TODO in the test ;-)
>
>> I have tested a fix that produces;
>>
>> var a:* = ((a + b) - (c + d)) * e;
>
> You are awesome! I might have to come visit you in your universe
> sometimes, I'm sure you over-hyper-cube-landers can teach me a trick
> or two.
>
> Looking forward to you commit,
>
> Have fun!
>
> EdB
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Erik de Bruin <er...@ixsoftware.nl>.
> You're not think 5th dimensionally here;

I rarely venture in such an integer state of mind. I'm more of a
fractal kinda guy :-)

> Currently without fixing this bug a use has the expression statement of;
>
> var a = ((a + b) - (c + d)) * e;
>
> They use the compiler, it checks out fine, FalconJx is currently rendering
> to JavaScript that will get executed;
>
> var a = a + b - c + d * e;
>
> See a problem here?

Yes, that's why I wrote you that TODO in the test ;-)

> I have tested a fix that produces;
>
> var a:* = ((a + b) - (c + d)) * e;

You are awesome! I might have to come visit you in your universe
sometimes, I'm sure you over-hyper-cube-landers can teach me a trick
or two.

Looking forward to you commit,

Have fun!

EdB



-- 
Ix Multimedia Software

Jan Luykenstraat 27
3521 VB Utrecht

T. 06-51952295
I. www.ixsoftware.nl

Re: [FalconJx] binary operator parenthesis

Posted by Michael Schmalle <ap...@teotigraphix.com>.
Erik,

You're not think 5th dimensionally here;

Currently without fixing this bug a use has the expression statement of;


var a = ((a + b) - (c + d)) * e;

They use the compiler, it checks out fine, FalconJx is currently  
rendering to JavaScript that will get executed;

var a = a + b - c + d * e;

See a problem here?

I have tested a fix that produces;

var a:* = ((a + b) - (c + d)) * e;

I have to fix all sub emitters that override emitBinaryOperator().

Fortunately it looks like we didn't add a lot of test code that relies  
on operator precedence so the test code to change looks not that  
difficult to update.

Mike



Quoting Erik de Bruin <er...@ixsoftware.nl>:

> Agreed (on the 'simple' fix).
>
> A thought: isn't doing this in Jx not somewhat over the top? I mean,
> if you are going to 'calculate' the parenthesis somehow, you might be
> fixing the user's code - after all, the user can put them in however
> he likes... Isn't there a way for the parenthesis to be (and this is
> probably going to sound silly, but I'm not so deep into this stuff
> that I know the actual jargon) part of the AST, so we can just 'visit'
> and 'emit' them?
>
> EdB
>
>
>
> On Wed, Mar 6, 2013 at 4:37 PM, Michael Schmalle
> <ap...@teotigraphix.com> wrote:
>> Don't worry about this one.
>>
>> When I fix this, I'm just going to have to get every test working again
>> before I commit. I have time since this is a breaking bug that will render
>> complex binary expressions useless as it stands.
>>
>> What I plan on doing is the most simple fix. Since I know that the tree is
>> correct, I will probably implement this in the before and after strategy to
>> wrap any binary expression that is an operator in ( ). This might cause some
>> extra parenthesis that "might" not be needed but for now I see this as the
>> safest and most accurate way to proceed.
>>
>> Mike
>>
>>
>> Quoting Erik de Bruin <er...@ixsoftware.nl>:
>>
>>> Mike,
>>>
>>> Sure, no problem. I'm not planning any work on Jx the next couple of
>>> days, so if you figure something in that time, go ahead and implement.
>>> Also, if you need help fixing tests, I don't mind, so feel free to
>>> ask.
>>>
>>> EdB
>>>
>>>
>>>
>>> On Wed, Mar 6, 2013 at 4:22 PM, Michael Schmalle
>>> <ap...@teotigraphix.com> wrote:
>>>>
>>>>
>>>> Erik,
>>>>
>>>> Now that I'm getting back into the core framework again I am wishing that
>>>> I
>>>> would have taken care of a bug that I knew about.
>>>>
>>>> There is a serious problem that has to be fixed. Since we don't save
>>>> tokens
>>>> from the parser, a Binary AST tree knows nothing about it's order of
>>>> operations when output using the visitor framework.
>>>>
>>>> The fortunate thing here is that when the ASParser assembles these nodes,
>>>> they are assembled based on operator precedence.
>>>>
>>>> The unfortunate thing is, when I go to fix this its more than likely
>>>> going
>>>> to affect a huge amount of tests.
>>>>
>>>> My plan is to create tests based on my other project so I stay out of the
>>>> main framework for now. When I figure out something that works I will let
>>>> you know because we should again be on the same revision so there aren't
>>>> hard merge issues.
>>>>
>>>> Mike
>>>>
>>>> --
>>>> Michael Schmalle - Teoti Graphix, LLC
>>>> http://www.teotigraphix.com
>>>> http://blog.teotigraphix.com
>>>>
>>>
>>>
>>>
>>> --
>>> Ix Multimedia Software
>>>
>>> Jan Luykenstraat 27
>>> 3521 VB Utrecht
>>>
>>> T. 06-51952295
>>> I. www.ixsoftware.nl
>>>
>>
>> --
>> Michael Schmalle - Teoti Graphix, LLC
>> http://www.teotigraphix.com
>> http://blog.teotigraphix.com
>>
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Erik de Bruin <er...@ixsoftware.nl>.
Agreed (on the 'simple' fix).

A thought: isn't doing this in Jx not somewhat over the top? I mean,
if you are going to 'calculate' the parenthesis somehow, you might be
fixing the user's code - after all, the user can put them in however
he likes... Isn't there a way for the parenthesis to be (and this is
probably going to sound silly, but I'm not so deep into this stuff
that I know the actual jargon) part of the AST, so we can just 'visit'
and 'emit' them?

EdB



On Wed, Mar 6, 2013 at 4:37 PM, Michael Schmalle
<ap...@teotigraphix.com> wrote:
> Don't worry about this one.
>
> When I fix this, I'm just going to have to get every test working again
> before I commit. I have time since this is a breaking bug that will render
> complex binary expressions useless as it stands.
>
> What I plan on doing is the most simple fix. Since I know that the tree is
> correct, I will probably implement this in the before and after strategy to
> wrap any binary expression that is an operator in ( ). This might cause some
> extra parenthesis that "might" not be needed but for now I see this as the
> safest and most accurate way to proceed.
>
> Mike
>
>
> Quoting Erik de Bruin <er...@ixsoftware.nl>:
>
>> Mike,
>>
>> Sure, no problem. I'm not planning any work on Jx the next couple of
>> days, so if you figure something in that time, go ahead and implement.
>> Also, if you need help fixing tests, I don't mind, so feel free to
>> ask.
>>
>> EdB
>>
>>
>>
>> On Wed, Mar 6, 2013 at 4:22 PM, Michael Schmalle
>> <ap...@teotigraphix.com> wrote:
>>>
>>>
>>> Erik,
>>>
>>> Now that I'm getting back into the core framework again I am wishing that
>>> I
>>> would have taken care of a bug that I knew about.
>>>
>>> There is a serious problem that has to be fixed. Since we don't save
>>> tokens
>>> from the parser, a Binary AST tree knows nothing about it's order of
>>> operations when output using the visitor framework.
>>>
>>> The fortunate thing here is that when the ASParser assembles these nodes,
>>> they are assembled based on operator precedence.
>>>
>>> The unfortunate thing is, when I go to fix this its more than likely
>>> going
>>> to affect a huge amount of tests.
>>>
>>> My plan is to create tests based on my other project so I stay out of the
>>> main framework for now. When I figure out something that works I will let
>>> you know because we should again be on the same revision so there aren't
>>> hard merge issues.
>>>
>>> Mike
>>>
>>> --
>>> Michael Schmalle - Teoti Graphix, LLC
>>> http://www.teotigraphix.com
>>> http://blog.teotigraphix.com
>>>
>>
>>
>>
>> --
>> Ix Multimedia Software
>>
>> Jan Luykenstraat 27
>> 3521 VB Utrecht
>>
>> T. 06-51952295
>> I. www.ixsoftware.nl
>>
>
> --
> Michael Schmalle - Teoti Graphix, LLC
> http://www.teotigraphix.com
> http://blog.teotigraphix.com
>



--
Ix Multimedia Software

Jan Luykenstraat 27
3521 VB Utrecht

T. 06-51952295
I. www.ixsoftware.nl

Re: [FalconJx] binary operator parenthesis

Posted by Michael Schmalle <ap...@teotigraphix.com>.
Don't worry about this one.

When I fix this, I'm just going to have to get every test working  
again before I commit. I have time since this is a breaking bug that  
will render complex binary expressions useless as it stands.

What I plan on doing is the most simple fix. Since I know that the  
tree is correct, I will probably implement this in the before and  
after strategy to wrap any binary expression that is an operator in (  
). This might cause some extra parenthesis that "might" not be needed  
but for now I see this as the safest and most accurate way to proceed.

Mike

Quoting Erik de Bruin <er...@ixsoftware.nl>:

> Mike,
>
> Sure, no problem. I'm not planning any work on Jx the next couple of
> days, so if you figure something in that time, go ahead and implement.
> Also, if you need help fixing tests, I don't mind, so feel free to
> ask.
>
> EdB
>
>
>
> On Wed, Mar 6, 2013 at 4:22 PM, Michael Schmalle
> <ap...@teotigraphix.com> wrote:
>>
>> Erik,
>>
>> Now that I'm getting back into the core framework again I am wishing that I
>> would have taken care of a bug that I knew about.
>>
>> There is a serious problem that has to be fixed. Since we don't save tokens
>> from the parser, a Binary AST tree knows nothing about it's order of
>> operations when output using the visitor framework.
>>
>> The fortunate thing here is that when the ASParser assembles these nodes,
>> they are assembled based on operator precedence.
>>
>> The unfortunate thing is, when I go to fix this its more than likely going
>> to affect a huge amount of tests.
>>
>> My plan is to create tests based on my other project so I stay out of the
>> main framework for now. When I figure out something that works I will let
>> you know because we should again be on the same revision so there aren't
>> hard merge issues.
>>
>> Mike
>>
>> --
>> Michael Schmalle - Teoti Graphix, LLC
>> http://www.teotigraphix.com
>> http://blog.teotigraphix.com
>>
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>

-- 
Michael Schmalle - Teoti Graphix, LLC
http://www.teotigraphix.com
http://blog.teotigraphix.com


Re: [FalconJx] binary operator parenthesis

Posted by Erik de Bruin <er...@ixsoftware.nl>.
Mike,

Sure, no problem. I'm not planning any work on Jx the next couple of
days, so if you figure something in that time, go ahead and implement.
Also, if you need help fixing tests, I don't mind, so feel free to
ask.

EdB



On Wed, Mar 6, 2013 at 4:22 PM, Michael Schmalle
<ap...@teotigraphix.com> wrote:
>
> Erik,
>
> Now that I'm getting back into the core framework again I am wishing that I
> would have taken care of a bug that I knew about.
>
> There is a serious problem that has to be fixed. Since we don't save tokens
> from the parser, a Binary AST tree knows nothing about it's order of
> operations when output using the visitor framework.
>
> The fortunate thing here is that when the ASParser assembles these nodes,
> they are assembled based on operator precedence.
>
> The unfortunate thing is, when I go to fix this its more than likely going
> to affect a huge amount of tests.
>
> My plan is to create tests based on my other project so I stay out of the
> main framework for now. When I figure out something that works I will let
> you know because we should again be on the same revision so there aren't
> hard merge issues.
>
> Mike
>
> --
> Michael Schmalle - Teoti Graphix, LLC
> http://www.teotigraphix.com
> http://blog.teotigraphix.com
>



--
Ix Multimedia Software

Jan Luykenstraat 27
3521 VB Utrecht

T. 06-51952295
I. www.ixsoftware.nl