I meant that this comment is exactly the substance that I was looking for.
- Josh
On 2019/02/19 20:59:30, Alex Harui <aha...@adobe.com.INVALID> wrote:
> What kind of substance are you looking for? Did you see the isXMLIsh() code?
>
> -Alex
>
> On 2/19/19, 12:08 PM, "Josh Tynjala" <joshtynj...@apache.org> wrote:
>
> I discovered the following comment in IdentifierNode.resolveMemberRef():
>
> // If the base type is XML or XMLList,
> // don't resolve a member reference to any definition.
> // As in the old compiler, this avoids possibly bogus
> // -strict type-coercion errors.
> // For example, we don't want a declared property like .name
> // to resolve to the method slot, because it might actually
> // be referring to a dynamically-defined XML attribute or child tag.
> // And if we did resolve it to the name() method, which returns Object,
> // then when doing s = x.name() where s is type String
> // and x is type XML you would get a can't-convert-Object-to-String
> // problem, but there is lots of existing source code that expects
> // this to compile with no cast.
>
> In short, types of XML members are not resolved on purpose.
>
> That's basically what I expected, but I thought it would be best to find
> something of substance in the compiler code or comments, if I could.
>
> As I mentioned before, the emitter should be able to make some more
> intelligent choices about what can be resolved, since it's closer to run-time.
>
> I just tested the following code in Flash:
>
> var xml:XML = <root>
> <toXMLString>hello</toXMLString>
> </root>;
> trace(xml.toXMLString());
> trace(xml.toXMLString);
>
> The first trace() call prints the correct result of toXMLString(). The
> second trace() call prints the "hello" text inside the child element. It does
> not access a reference to the function. So, the emitter should have a special
> case for calling XML/XMLList methods, but not referencing them without a call.
>
> - Josh
>
> On 2019/02/19 16:03:05, Josh Tynjala <joshtynj...@apache.org> wrote:
> > 1. b.moveTo(Number(path.pathPoints[0].anchor.x),
> Number(path.pathPoints[0].anchor.y));
> >
> > I have determined that the compiler is not correctly resolving the type
> of pathPoints[0]. It should be PathPointVO because we're dealing with a
> Vector, but it's being resolved as * instead, which is how regular Arrays are
> handled.
> >
> > I can see that this issue with resolving the type also affects code
> intelligence in VSCode. Members of PathPointVO are not available in the
> completion list for pathPoints[0].
> >
> > I have checked Flash Builder, and the type of a Vector item seems to be
> known because the correct completion items are suggested. Additionally, a
> compiler error is created for the following code in Flash Builder, but not
> from Royale:
> >
> > var firstItem:SomeOtherType = pathPoints[0];
> >
> > > Error: Implicit coercion of a value with static type PathPointVO to a
> possibly unrelated type SomeOtherType.
> >
> > This indicates to me that Adobe fixed this issue in ASC 2.0 after the
> donation of "Falcon" to Apache, and we should do the same.
> >
> > Once the correct type is resolved here, there won't be any coercion
> emitted.
> >
> > 2. paragraph.setStyle("tabXML",
> escape(org.apache.royale.utils.Language.string(para.tabXML.toXMLString())));
> >
> > Confirmed.
> >
> > Interestingly, in VSCode, toXMLString() is provided in the completion
> list, but hover and goto definition are not working on it. This indicates to
> me that the compiler is not correctly resolving toXMLString(), so it's
> falling back to * for the type.
> >
> > Also interestingly, Flash Builder provides hover for toXMLString(), but
> no compiler error when assigning to another type. I suspect that XML is
> getting special treatment in FB, and ASC 2.0 does not resolve the type of
> anything on XML either.
> >
> > It looks like the emitter needs a special case for XML and I can
> manually resolve certain types. There may be some logic for this already in
> one of the emitters. I'll make sure that it works the same everywhere.
> >
> > 3.
> doc.retrieveFile(org.apache.royale.utils.Language.string(file.attribute('loc').toString()));
> >
> > This is basically the same as 2. Lack of type resolution for XML.
> >
> > 4. stroke.setLineStyle(1, 65535, 1);
> >
> > This was a result of ensuring that numeric literals are emitted
> correctly for integers. The value is the same, but it's just formatted a
> different way. I think that I had the option of emitting the literal with a
> specific radix, so I may be able to detect and preserve the 0x formatting.
> >
> > 5.
> xml.setNodeKind(org.apache.royale.utils.Language.string(this.XML__nodeKind));
> >
> > This looks to be the same as 2 and 3. Again, lack of type resolution
> for XML.
> >
> > - Josh
> >
> > On 2019/02/10 10:02:53, Harbs <harbs.li...@gmail.com> wrote:
> > > The commit seems to break some things for me.
> > >
> > > I’m working on finding the exact problem, but I’m noticing issues
> along the way:
> > >
> > > 1.
> > > In function:
> > > function drawPath(path:BezierPathVO,b:PathBuilder):void
> > >
> > > The following:
> > > b.moveTo(path.pathPoints[0].anchor.x,path.pathPoints[0].anchor.y);
> > >
> > > compiles to:
> > > b.moveTo(Number(path.pathPoints[0].anchor.x),
> Number(path.pathPoints[0].anchor.y));
> > >
> > > pathPoints is of type Vector.<PathPointVO>
> > > and PathPointVO anchor is of type Point
> > >
> > > x and y should be inferrable that they are Numbers and it should
> compile unchanged.
> > >
> > > 2.
> > > paragraph.setStyle("tabXML", escape(para.tabXML.toXMLString()));
> > >
> > > Compiles to:
> > > paragraph.setStyle("tabXML",
> escape(org.apache.royale.utils.Language.string(para.tabXML.toXMLString())));
> > >
> > > para.tabXML is typed as XML and toXMLString() should be recognizable
> as a String and Language.string() should not be called.
> > >
> > > 3.
> > > var file:XML = files[i];
> > > doc.retrieveFile(file.@loc.toString());
> > >
> > > Compiles to:
> > >
> doc.retrieveFile(org.apache.royale.utils.Language.string(file.attribute('loc').toString()));
> > >
> > > The compiler should know that toString() is a string and doesn’t need
> Language.string()
> > >
> > > 4.
> > > Another interesting change:
> > > stroke.setLineStyle(1,0x00FFFF,1);
> > >
> > > Now becomes:
> > > stroke.setLineStyle(1, 65535, 1);
> > >
> > > I’m not sure whether this is something we should be concerned about
> or not.
> > >
> > > 5.
> > > XML has many cases where you have something like this:
> > > xml.setNodeKind(_nodeKind);
> > > becomes
> > >
> xml.setNodeKind(org.apache.royale.utils.Language.string(this.XML__nodeKind));
> > >
> > > Although everything is strings…
> > >
> > >
> > >
> > >
> > >
> > >
> > > > On Feb 6, 2019, at 6:40 PM, joshtynj...@apache.org wrote:
> > > >
> > > > This is an automated email from the ASF dual-hosted git repository.
> > > >
> > > > joshtynjala pushed a commit to branch develop
> > > > in repository
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.git&data=02%7C01%7Caharui%40adobe.com%7C11e86e4784fc43331e1208d696a60614%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636862037140997434&sdata=Xx1OzMKWtnUXuVKKyGZt4inrO%2FS6mv4lOW66w0hztLU%3D&reserved=0
> > > >
> > > > commit fd7b81f4448db0f5eb70f22208c9144549cc4806
> > > > Author: Josh Tynjala <joshtynj...@apache.org>
> > > > AuthorDate: Wed Feb 6 08:36:10 2019 -0800
> > > >
> > > > ParametersEmitter: fixed automatic type coercion and added some
> tests (closes #74)
> > > > ---
> > > > .../js/jx/FunctionCallArgumentsEmitter.java | 10 +++++--
> > > > .../codegen/js/royale/TestRoyaleExpressions.java | 34
> +++++++++++++++++++++-
> > > > 2 files changed, 40 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallArgumentsEmitter.java
>
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallArgumentsEmitter.java
> > > > index 40b6302..c92f189 100644
> > > > ---
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallArgumentsEmitter.java
> > > > +++
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallArgumentsEmitter.java
> > > > @@ -61,17 +61,21 @@ public class FunctionCallArgumentsEmitter
> extends JSSubEmitter implements
> > > > for (int i = 0; i < len; i++)
> > > > {
> > > > IExpressionNode argumentNode = (IExpressionNode)
> node.getChild(i);
> > > > - IParameterDefinition paramDef = null;
> > > > + IDefinition paramTypeDef = null;
> > > > if (paramDefs != null && paramDefs.length > i)
> > > > {
> > > > - paramDef = paramDefs[i];
> > > > + IParameterDefinition paramDef = paramDefs[i];
> > > > if (paramDef.isRest())
> > > > {
> > > > paramDef = null;
> > > > }
> > > > + if (paramDef != null)
> > > > + {
> > > > + paramTypeDef =
> paramDef.resolveType(getProject());
> > > > + }
> > > > }
> > > >
> > > > - getEmitter().emitAssignmentCoercion(argumentNode,
> paramDef);
> > > > + getEmitter().emitAssignmentCoercion(argumentNode,
> paramTypeDef);
> > > >
> > > > if (i < len - 1)
> > > > {
> > > > diff --git
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
>
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
> > > > index 93db140..56b6363 100644
> > > > ---
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
> > > > +++
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
> > > > @@ -1523,7 +1523,7 @@ public class TestRoyaleExpressions extends
> TestGoogExpressions
> > > > @Test
> > > > public void testVisitCallFunctionReturnedFromFunction()
> > > > {
> > > > - IFunctionCallNode node = (IFunctionCallNode)
> getNode("function foo(a:String, b:String):Function { return null }; return
> foo(3, 4)(1, 2);",
> > > > + IFunctionCallNode node = (IFunctionCallNode)
> getNode("function foo(a:int, b:int):Function { return null }; return foo(3,
> 4)(1, 2);",
> > > >
> IFunctionCallNode.class);
> > > > asBlockWalker.visitFunctionCall(node);
> > > > assertOut("foo(3, 4)(1, 2)");
> > > > @@ -1571,6 +1571,38 @@ public class TestRoyaleExpressions extends
> TestGoogExpressions
> > > > assertOut("return 4294967173");
> > > > }
> > > >
> > > > + @Test
> > > > + public void testVisitFunctionCallWithIntParameterNegative()
> > > > + {
> > > > + IFunctionCallNode node = (IFunctionCallNode)
> getNode("function a(foo:int):void {}; a(-123)", IFunctionCallNode.class);
> > > > + asBlockWalker.visitFunctionCall(node);
> > > > + assertOut("a(-123)");
> > > > + }
> > > > +
> > > > + @Test
> > > > + public void testVisitFunctionCallWithIntParameterDecimal()
> > > > + {
> > > > + IFunctionCallNode node = (IFunctionCallNode)
> getNode("function a(foo:int):void {}; a(123.4)", IFunctionCallNode.class);
> > > > + asBlockWalker.visitFunctionCall(node);
> > > > + assertOut("a(123)");
> > > > + }
> > > > +
> > > > + @Test
> > > > + public void testVisitFunctionCallWithUintParameterNegative()
> > > > + {
> > > > + IFunctionCallNode node = (IFunctionCallNode)
> getNode("function a(foo:uint):void {}; a(-123)", IFunctionCallNode.class);
> > > > + asBlockWalker.visitFunctionCall(node);
> > > > + assertOut("a(4294967173)");
> > > > + }
> > > > +
> > > > + @Test
> > > > + public void testVisitFunctionCallWithUintParameterDecimal()
> > > > + {
> > > > + IFunctionCallNode node = (IFunctionCallNode)
> getNode("function a(foo:uint):void {}; a(123.4)", IFunctionCallNode.class);
> > > > + asBlockWalker.visitFunctionCall(node);
> > > > + assertOut("a(123)");
> > > > + }
> > > > +
> > > > protected IBackend createBackend()
> > > > {
> > > > return new RoyaleBackend();
> > > >
> > >
> > >
> >
>
>
>
