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" <[email protected]> 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 <[email protected]> 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" <[email protected]> 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 <[email protected]> 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" <[email protected]> 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 >> > > >
