My latest improvements to the compiler should resolve these 5 issues with
coercion.
- 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://gitbox.apache.org/repos/asf/royale-compiler.git
> > >
> > > 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();
> > >
> >
> >
>
