royale-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Harbs <harbs.li...@gmail.com>
Subject Re: XML filters broke
Date Wed, 08 Aug 2018 20:31:24 GMT
Are you sure the logic to reassign this will work here?

I’m willing to rewrite the code in XMLList to use call if you think it’ll make things
easier in the compiler…

> On Aug 8, 2018, at 11:03 PM, Alex Harui <aharui@adobe.com.INVALID> wrote:
> 
> 
> 
> On 8/8/18, 12:59 AM, "Harbs" <harbs.lists@gmail.com <mailto:harbs.lists@gmail.com>>
wrote:
> 
>    Does “this” in call/apply even work for a function which is not a prototype function?
I thought in that case “this” is the global context.
> 
> From my testing, the 'this' can be re-assigned as we want it.
> 
>    I think 6a is kind of ambiguous. Why do you think there’s a difference between the
following (other than avoiding ambiguous this references)?
> 
> Because there is already code that distinguishes when 'this' is supposed to be used.
 So we should use it instead of crafting a whole other set of code that has a more difficult
problem to solve, like whether an expression is relative to a parameter and if so, which parameter?
> 
> My 2 cents,
> -Alex
> 
>    function() { return (over40(parseInt(this.age))) }
>    and 
>    function(node) { return (over40(parseInt(node.age))) }
> 
>    Although in fact, I think it would need to be:
> 
>    function(node) { return (over40(parseInt(node.child(“age”)))) }
> 
>> On Aug 8, 2018, at 10:33 AM, Alex Harui <aharui@adobe.com.INVALID> wrote:
>> 
>> EmitterUtils.writeThis seems to be working ok.  I think it would be better/correct
to use it here.  I'm not sure if node as a parameter creates the same scope chain as node
being the this pointer.  I think no, I don't think parameters are involved in lexical scoping.
  IMO, 6a in the spec is saying we should make node the 'this' pointer in JS.
>> 
>> My 2 cents,
>> -Alex
>> 
>> On 8/7/18, 10:54 AM, "Harbs" <harbs.lists@gmail.com> wrote:
>> 
>>   I’m not following you. Wouldn’t we need the same logic to figure out where
to insert “this”? I’m not sure what practical difference there would be between “node"
and “this”, other than using apply or call. Passing in the XML node seems cleaner to me.
>> 
>>> On Aug 7, 2018, at 6:50 PM, Alex Harui <aharui@adobe.com.INVALID> wrote:
>>> 
>>> Yup.  After thinking about it some more, it occurs to me that we took the wrong
starting point.  Right now code like:
>>> 
>>> over40(parseInt(age))
>>> 
>>> Results in:
>>> 
>>> function(node) { return (over40(parseInt(age))) }
>>> 
>>> And then the XML filter calls that function passing itself in as the node.
>>> 
>>> And this discussion has been about trying to figure out where to add the "node"
parameter.  But now I think that 6a in the spec is really saying that the 'this' pointer should
be the node.  We should transpile that filter expression like any other function body but
assume it is a function run in the context of the node, like a new method on XML/XMLList,
or maybe more like an anonymous function.
>>> 
>>> So if I'm right, then the output should be:
>>> 
>>> function() { return (over40(parseInt(this.age))) }
>>> 
>>> This is what the transpiler would have output if you had subclassed XML and added
this method to it.  And then the code in XML.SWC that applies the filter has to use Function.apply/call
passing the node as the 'this' pointer.
>>> 
>>> If we do that, then the EmitterUtils.writeE4xFilterNode would go away, and JSRoyaleEmitter.emitE4xFilter
would temporarily change the model.currentClass and maybe a few other things to reference
an XML object.
>>> 
>>> Thoughts?
>>> -Alex
>>> 
>>> PS: Regarding adding tests, the filter tests have two variables.  "var a" sets
up the XML, "var b" is the result of the filter.  getVariable returns the nodes for "a" then
we go get child(1) which is "b" and then emit it to see what we get.
>>> 
>>> On 8/7/18, 3:51 AM, "Harbs" <harbs.lists@gmail.com <mailto:harbs.lists@gmail.com>>
wrote:
>>> 
>>>  I’m also pretty sure that the following from Mozilla’s page[1] will not
work correctly:
>>> 
>>>  var people = <people>
>>>    <person>
>>>      <name>Bob</name>
>>>      <age>32</age>
>>>    </person>
>>>    <person>
>>>      <name>Joe</name>
>>>      <age>46</age>
>>>    </person>
>>>  </people>;
>>> 
>>>  function over40(i) {
>>>      return i > 40;
>>>  }
>>> 
>>>  alert(people.person.(over40(parseInt(age))).name); // Alerts Joe
>>> 
>>>  https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0>
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0>>
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0>
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FArchive%2FWeb%2FE4X%2FProcessing_XML_with_E4X&amp;data=02%7C01%7Caharui%40adobe.com%7C441a59794e0f493d1c7b08d5fd04d3f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636693119531996112&amp;sdata=i5AMXcWmppuFIqNky5A5a3A3m9NG2x30NrzCxDw37Ss%3D&amp;reserved=0>>>
>>> 
>>>> On Aug 7, 2018, at 1:41 PM, Harbs <harbs.lists@gmail.com> wrote:
>>>> 
>>>> OK. I fixed the issue, but there’s a couple of loose ends:
>>>> 
>>>> 1. I don’t know how to add unit tests for these cases. In the current unit
tests, I see “getNode” and “getVariable” being used. I don’t know the logic in setting
up tests.
>>>> 2. I’m not quite sure what "parentNode.getChild(0)” does. What is the
parent node and will this cause my second case of e.employee.(1 == @id) to fail? Removing
the check against firstChild caused the testXMLFilterWithAttribute test to fail because it
prepended “node.” to “length()”.
>>>> 
>>>> P.S. I finally got debugging from Eclipse working on the compiler, so hopefully
I’ll have a much easier time fixing compiler issues in the future. :-)
>>>> 
>>>> Thanks,
>>>> Harbs
>>>> 
>>>>> On Aug 7, 2018, at 10:51 AM, Harbs <harbs.lists@gmail.com> wrote:
>>>>> 
>>>>> Well, it looks like I was wrong.
>>>>> 
>>>>> I just tested the following in Flash, and then both give the same results
(i.e. return the attribute):
>>>>> 
>>>>> var emp = e.employee.(@id == 1).@name; // name of employee with id 1
>>>>> var foo = e.employee.(1 == @id).@name; // name of employee with id 1
>>>>> 
>>>>>> On Aug 7, 2018, at 10:27 AM, Harbs <harbs.lists@gmail.com>
wrote:
>>>>>> 
>>>>>> Your example does not seem to be right to me.
>>>>>> 
>>>>>> Here’s the overview of how filters are supposed to work from the
spec:
>>>>>> 
>>>>>>> Overview
>>>>>>> When the left operand evaluates to an XML object, the filtering
predicate adds the left operand to the front of the scope chain of the current execution context,
evaluates the Expression with the augmented scope chain, converts the result to a Boolean
value, then restores the scope chain. If the result is true, the filtering predicate returns
an XMLList containing the left operand. Otherwise it returns an empty XMLList.
>>>>>>> When the left operand is an XMLList, the filtering predicate
is applied to each XML object in the XMLList in order using the XML object as the left operand
and the Expression as the right operand. It concatenates the results and returns them as a
single XMLList containing all the XML properties for which the result was true. For example,
>>>>>>> 
>>>>>>> var john = e.employee.(name == "John"); // employees with name
John
>>>>>>> var twoemployees = e.employee.(@id == 0 || @id == 1); // employees
with id's 0 & 1
>>>>>>> var emp = e.employee.(@id == 1).name; // name of employee with
id 1
>>>>>>> 
>>>>>>> The effect of the filtering predicate is similar to SQL’s WHERE
clause or XPath’s filtering predicates.
>>>>>>> For example, the statement:
>>>>>>> 
>>>>>>> // get the two employees with ids 0 and 1 using a predicate
>>>>>>> var twoEmployees = e..employee.(@id == 0 || @id == 1);
>>>>>>> 
>>>>>>> produces the same result as the following set of statements:
>>>>>>> // get the two employees with the ids 0 and 1 using a for loop
>>>>>>> var i = 0;
>>>>>>> var twoEmployees = new XMLList();
>>>>>>> for each (var p in e..employee) {
>>>>>>> with (p) {
>>>>>>> if (@id == 0 || @id == 1) {
>>>>>>> twoEmployees[i++] = p;
>>>>>>> }
>>>>>>> }
>>>>>>> }
>>>>>> 
>>>>>> The question is what is "the front of the scope chain of the current
execution context”? I’m pretty sure that means the start of sub-expressions. I don’t
see how that can apply to the right-hand of comparison expressions. There is nothing in the
spec about figuring out if a part of an expression is referring to XML or XMLList.
>>>>>> 
>>>>>>> On Aug 7, 2018, at 9:45 AM, Alex Harui <aharui@adobe.com.INVALID>
wrote:
>>>>>>> 
>>>>>>> I don't get what portion of the spec has to do with whether we
append "node" to various expressions.  IMO, the changes I made only affect 6b.  6a is handled
by generating a function with "node" as the parameter (because node is list[i] in the spec).
 The task in 6b is to correctly evaluate any e4x filter expression.  I'm not sure what the
limits are on what you can have in a filter expression, but if you can have just plain "@app"
anywhere in the filter expression, I don't believe scoping rules would know to apply that
to the "node" parameter without generating the "node" before "@app".
>>>>>>> 
>>>>>>> There is a chance that the Flex Compiler was using "magic" to
generate the "node" and really should have reported an error.  I do remember being told that
the filter function can be "anything".  Even:
>>>>>>> (var foo:int = @app.length(); foo > @bar.length())  
>>>>>>> 
>>>>>>> If there are actual rules in the spec about evaluating the expression,
that might apply to how we handle these expressions, otherwise I think the right thing is
to resolve each expression and if the expression does not resolve to anything else, assume
that it applies to the node.   I know the logic in EmitterUtils.writeE4xFilterNode isn't covering
all cases.  It is trying to see what the expression resolves to, and returns false for known
conditions (like a member of a class).  Just make it return false for your case (and feel
free to add that case to the tests).  Eventually we'll have enough cases to either call it
"good enough" or figure out a better way to determine when the expression applies to "node".
>>>>>>> 
>>>>>>> My 2 cents,
>>>>>>> -Alex
>>>>>>> 
>>>>>>> On 8/6/18, 11:20 PM, "Harbs" <harbs.lists@gmail.com> wrote:
>>>>>>> 
>>>>>>> I just looked at the spec. I think it’s correct to append “node”
to the first statement of the expression only. The only exception seems to be expressions
which use boolean expressions (i.e. || or &&) in which case each piece of the boolean
expression should be considered a self-contained expression. So in your example, there are
really two filter expressions:
>>>>>>> 1. hasOwnProperty("@app”)
>>>>>>> 2. @app.length() > 0
>>>>>>> 
>>>>>>> Both of those should have node appended to the front, but nothing
else.
>>>>>>> 
>>>>>>> Here’s the relevant semantics in the spec (the important bit
being 6a):
>>>>>>> 
>>>>>>>> 6. For i = 0 to list.[[Length]]-1
>>>>>>>> a. Add list[i] to the front of the scope chain
>>>>>>>> b. Let ref be the result of evaluating Expression using the
augmented scope chain of step 6a
>>>>>>>> c. Let match = ToBoolean(GetValue(ref))
>>>>>>>> d. Remove list[i] from the front of the scope chain
>>>>>>>> e. If (match == true), call the [[Append]] method of r with
argument list[i]
>>>>>>>> 7. Return r
>>>>>>> 
>>>>>>> Makes sense?
>>>>>>> 
>>>>>>> Harbs
>>>>>>> 
>>>>>>>> On Aug 7, 2018, at 1:39 AM, Alex Harui <aharui@adobe.com.INVALID>
wrote:
>>>>>>>> 
>>>>>>>> In porting Tour De Flex, there were patterns like this (explorerTree
is XML):
>>>>>>>> 
>>>>>>>> explorerTree..node.(hasOwnProperty("@app") && @app.length()
> 0)
>>>>>>>> 
>>>>>>>> The compiler logic before I made any changes yesterday just
assumed that the first expression was a reference to the node parameter but other expressions
were not, but it looks like the expression "@app.length()" was allowed in Flex as a reference
to the node.  So I think the compiler has to determine what expressions evaluate to "nothing"
which implies they are references to the node, and what did resolve to something.  This is
all new logic and I don't know how to determine all of the test cases up front, so we'll have
to keep tuning it as we find patterns that don't work as we want them to.
>>>>>>>> 
>>>>>>>> In your case, if the expression resolves to a VariableDefinition,
that probably means that isn't a reference to node.  Not exactly sure, so you should debug
into it to see what the node pattern is and return false.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> -Alex
>>>>>>>> 
>>>>>>>> On 8/6/18, 3:28 PM, "Harbs" <harbs.lists@gmail.com>
wrote:
>>>>>>>> 
>>>>>>>> Doesn’t it always need to be a method for it to reference
the node?
>>>>>>>> 
>>>>>>>> I.e. child() should be node.child(), but foo.baz would not.
>>>>>>>> 
>>>>>>>>> On Aug 7, 2018, at 1:12 AM, Alex Harui <aharui@adobe.com.INVALID>
wrote:
>>>>>>>>> 
>>>>>>>>> Yep, we need more intelligent understanding of when a
reference is to the node or not.
>>>>>>>>> 
>>>>>>>>> Debug into EmitterUtils.writeE4xFilterNode and figure
out the node pattern you need.
>>>>>>>>> 
>>>>>>>>> -Alex
>>>>>>>>> 
>>>>>>>>> On 8/6/18, 3:09 PM, "Harbs" <harbs.lists@gmail.com>
wrote:
>>>>>>>>> 
>>>>>>>>> var folderFolders:XMLList = assetXML.folder.(child('key').indexOf(folder.key)
== 0);
>>>>>>>>> var folderImages:XMLList = assetXML.image.(child('key').indexOf(folder.key)
== 0);
>>>>>>>>> 
>>>>>>>>> Is now compiled as:
>>>>>>>>> 
>>>>>>>>> var /** @type {XMLList} */ folderFolders = this.assetXML.child('folder').filter(function(node){return
(node.child('key').indexOf(node.folder.key) == 0)});
>>>>>>>>> var /** @type {XMLList} */ folderImages = this.assetXML.child('image').filter(function(node){return
(node.child('key').indexOf(node.folder.key) == 0)});
>>>>>>>>> 
>>>>>>>>> “node.folder.key” is not correct. “folder” is
a local variable of an un related object type.
>>>>>>>>> 
>>>>>>>>> I assume this broke with the recent XML filter changes.
>>>>>>>>> 
>>>>>>>>> Harbs


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message