You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@royale.apache.org by gr...@apache.org on 2022/07/11 05:25:39 UTC

[royale-compiler] branch develop updated: Compiler changes to support various XMLish String coercions wihch avoid Language.string and other cases which cause implicit valueOf() calls - forcing toString() calls instead in relevant scenarios.

This is an automated email from the ASF dual-hosted git repository.

gregdove pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/royale-compiler.git


The following commit(s) were added to refs/heads/develop by this push:
     new 18cb7cd27 Compiler changes to support various XMLish String coercions wihch avoid Language.string and other cases which cause implicit valueOf() calls - forcing toString() calls instead in relevant scenarios.
18cb7cd27 is described below

commit 18cb7cd27a8c1bd63069f6c6067f8b0dc9274906
Author: greg-dove <gr...@gmail.com>
AuthorDate: Mon Jul 11 17:25:29 2022 +1200

    Compiler changes to support various XMLish String coercions wihch avoid Language.string and other cases which cause implicit valueOf() calls - forcing toString() calls instead in relevant scenarios.
---
 .../compiler/internal/codegen/js/JSEmitter.java    | 45 +++++++----------
 .../codegen/js/jx/BinaryOperatorEmitter.java       | 56 ++++++++++++++++++++++
 .../codegen/js/jx/FunctionCallEmitter.java         | 30 +++++++++++-
 .../internal/codegen/js/utils/EmitterUtils.java    | 43 +++++++++++++++++
 .../codegen/js/royale/TestRoyaleExpressions.java   |  6 +--
 .../codegen/js/royale/TestRoyaleGlobalClasses.java |  4 +-
 6 files changed, 149 insertions(+), 35 deletions(-)

diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSEmitter.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSEmitter.java
index 6ba481ca3..85f069c5e 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSEmitter.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSEmitter.java
@@ -68,38 +68,13 @@ import org.apache.royale.compiler.internal.codegen.js.jx.WhileLoopEmitter;
 import org.apache.royale.compiler.internal.codegen.js.jx.WithEmitter;
 import org.apache.royale.compiler.internal.codegen.js.royale.JSRoyaleDocEmitter;
 import org.apache.royale.compiler.internal.codegen.js.royale.JSRoyaleEmitterTokens;
+import org.apache.royale.compiler.internal.codegen.js.utils.EmitterUtils;
 import org.apache.royale.compiler.internal.definitions.ParameterDefinition;
 import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
 import org.apache.royale.compiler.internal.semantics.SemanticUtils;
 import org.apache.royale.compiler.internal.tree.as.*;
 import org.apache.royale.compiler.projects.ICompilerProject;
-import org.apache.royale.compiler.tree.as.IASNode;
-import org.apache.royale.compiler.tree.as.ICatchNode;
-import org.apache.royale.compiler.tree.as.IContainerNode;
-import org.apache.royale.compiler.tree.as.IDefinitionNode;
-import org.apache.royale.compiler.tree.as.IDynamicAccessNode;
-import org.apache.royale.compiler.tree.as.IExpressionNode;
-import org.apache.royale.compiler.tree.as.IForLoopNode;
-import org.apache.royale.compiler.tree.as.IFunctionNode;
-import org.apache.royale.compiler.tree.as.IFunctionObjectNode;
-import org.apache.royale.compiler.tree.as.IIfNode;
-import org.apache.royale.compiler.tree.as.IImportNode;
-import org.apache.royale.compiler.tree.as.IIterationFlowNode;
-import org.apache.royale.compiler.tree.as.ILanguageIdentifierNode;
-import org.apache.royale.compiler.tree.as.ILiteralContainerNode;
-import org.apache.royale.compiler.tree.as.INumericLiteralNode;
-import org.apache.royale.compiler.tree.as.IObjectLiteralValuePairNode;
-import org.apache.royale.compiler.tree.as.IParameterNode;
-import org.apache.royale.compiler.tree.as.IReturnNode;
-import org.apache.royale.compiler.tree.as.ISwitchNode;
-import org.apache.royale.compiler.tree.as.ITernaryOperatorNode;
-import org.apache.royale.compiler.tree.as.IThrowNode;
-import org.apache.royale.compiler.tree.as.ITryNode;
-import org.apache.royale.compiler.tree.as.ITypeNode;
-import org.apache.royale.compiler.tree.as.ITypedExpressionNode;
-import org.apache.royale.compiler.tree.as.IUnaryOperatorNode;
-import org.apache.royale.compiler.tree.as.IWhileLoopNode;
-import org.apache.royale.compiler.tree.as.IWithNode;
+import org.apache.royale.compiler.tree.as.*;
 
 import com.google.debugging.sourcemap.FilePosition;
 
@@ -549,18 +524,22 @@ public class JSEmitter extends ASEmitter implements IJSEmitter
         IDefinition assignedDef = null;
         IDefinition assignedTypeDef = null;
         ICompilerProject project = getWalker().getProject();
+        boolean isXML = false;
         if (assignedNode != null)
         {
             assignedDef = assignedNode.resolve(project);
             assignedTypeDef = assignedNode.resolveType(project);
-            if (project.getBuiltinType(BuiltinType.ANY_TYPE).equals(assignedTypeDef))
+            if (assignedTypeDef == null || project.getBuiltinType(BuiltinType.ANY_TYPE).equals(assignedTypeDef))
             {
                 IDefinition resolvedXMLDef = SemanticUtils.resolveXML(assignedNode, project);
                 if (resolvedXMLDef != null)
                 {
                     assignedDef = resolvedXMLDef;
                     assignedTypeDef = SemanticUtils.resolveTypeXML(assignedNode, project);
+                    isXML = true;
                 }
+            } else if (SemanticUtils.isXMLish(assignedTypeDef, project)) {
+                isXML = true;
             }
         }
 		String coercionStart = null;
@@ -724,7 +703,15 @@ public class JSEmitter extends ASEmitter implements IJSEmitter
             }
             if (emitStringCoercion)
             {
-                coercionStart = "org.apache.royale.utils.Language.string(";
+                if (isXML) {
+                    if (EmitterUtils.xmlRequiresNullCheck((NodeBase) assignedNode, project)) {
+                        //if it could be a null reference use the XMLList.coerce_string method, which retains null
+                        coercionStart = "XMLList.coerce_string(";
+                    } else {
+                        coercionStart = "String(";
+                    }
+                }
+                else coercionStart = "org.apache.royale.utils.Language.string(";
             }
         }
         if ( assignedDef != null
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/BinaryOperatorEmitter.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/BinaryOperatorEmitter.java
index 38a3f3e27..5a1890d46 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/BinaryOperatorEmitter.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/BinaryOperatorEmitter.java
@@ -34,6 +34,7 @@ import org.apache.royale.compiler.internal.codegen.js.JSSubEmitter;
 import org.apache.royale.compiler.internal.codegen.js.royale.JSRoyaleEmitter;
 import org.apache.royale.compiler.internal.codegen.js.royale.JSRoyaleEmitterTokens;
 import org.apache.royale.compiler.internal.codegen.js.goog.JSGoogEmitterTokens;
+import org.apache.royale.compiler.internal.codegen.js.utils.EmitterUtils;
 import org.apache.royale.compiler.internal.definitions.AccessorDefinition;
 import org.apache.royale.compiler.internal.definitions.AppliedVectorDefinition;
 import org.apache.royale.compiler.internal.definitions.NamespaceDefinition;
@@ -601,6 +602,61 @@ public class BinaryOperatorEmitter extends JSSubEmitter implements
 						}
 					}
 				}
+			} else if (id == ASTNodeID.Op_AddID) {
+				IDefinition rightDef = node.getRightOperandNode().resolveType(getProject());
+				boolean leftIsXMLish = (SemanticUtils.isXMLish(node.getLeftOperandNode(), getProject())) || SemanticUtils.isXMLish(leftDef, getProject());
+				boolean rightIsXMLish = (SemanticUtils.isXMLish(node.getRightOperandNode(), getProject())) || SemanticUtils.isXMLish(rightDef, getProject());
+				boolean process;
+				if (leftIsXMLish) {
+					process = !rightIsXMLish;
+				} else {
+					process = rightIsXMLish;
+				}
+				if (process) {
+					IASNode codeContext = node.getAncestorOfType(IClassNode.class);
+					boolean isFrameworkXML = false;
+					if (codeContext instanceof IClassNode) {
+						if (((IClassNode) codeContext).getQualifiedName().equals("XML") || ((IClassNode) codeContext).getQualifiedName().equals("XMLList")) {
+							//we will ignore the internal code of the emulation support classes for these cases
+							isFrameworkXML = true;
+						}
+					}
+					if (!isFrameworkXML) {
+						IExpressionNode leftOperand = node.getLeftOperandNode();
+						IExpressionNode rightOperand = node.getRightOperandNode();
+						FunctionCallNode functionCallNode;
+						if (leftIsXMLish) {
+							//wrap in string coercion
+							if (EmitterUtils.xmlRequiresNullCheck((NodeBase) leftOperand, getProject())) {
+								//if it is a simple identifier, then it could be a null reference so use the XMLList.coerce_string method, which retains null
+								functionCallNode = EmitterUtils.wrapXMLListStringCoercion((NodeBase) leftOperand);
+							} else {
+								//if it is a member access expression or something else then assume we don't have to check for null
+								functionCallNode = EmitterUtils.wrapSimpleStringCoercion((NodeBase) leftOperand);
+							}
+							functionCallNode.setParent((NodeBase) node);
+							leftOperand = functionCallNode;
+						} else {
+							//wrap in string coercion
+							if (EmitterUtils.xmlRequiresNullCheck((NodeBase) rightOperand, getProject())) {
+								//if it is a simple identifier, then it could be a null reference so use the XMLList.coerce_string method, which retains null
+								functionCallNode = EmitterUtils.wrapXMLListStringCoercion((NodeBase) rightOperand);
+							} else {
+								//if it is a member access expression or something else then assume we don't have to check for null
+								functionCallNode = EmitterUtils.wrapSimpleStringCoercion((NodeBase) rightOperand);
+							}
+							functionCallNode.setParent((NodeBase) node);
+							rightOperand = functionCallNode;
+						}
+						getWalker().walk(leftOperand);
+						write(ASEmitterTokens.SPACE);
+						writeToken(ASEmitterTokens.PLUS);
+						write(ASEmitterTokens.SPACE);
+						getWalker().walk(rightOperand);
+						return;
+					}
+				}
+
 			}
 			
 		
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallEmitter.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallEmitter.java
index 3653baeb9..75e478d83 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallEmitter.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/FunctionCallEmitter.java
@@ -39,6 +39,7 @@ import org.apache.royale.compiler.internal.codegen.js.utils.EmitterUtils;
 import org.apache.royale.compiler.internal.definitions.*;
 import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
 import org.apache.royale.compiler.internal.scopes.FunctionScope;
+import org.apache.royale.compiler.internal.semantics.SemanticUtils;
 import org.apache.royale.compiler.internal.tree.as.*;
 import org.apache.royale.compiler.problems.ProjectSpecificErrorProblem;
 import org.apache.royale.compiler.problems.TooFewFunctionParametersProblem;
@@ -68,6 +69,7 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
             cnode = cnode.getChild(0);
         String postCallAppend = null;
         ASTNodeID id = cnode.getNodeID();
+        boolean numericCast = false;
         if (id != ASTNodeID.SuperID)
         {
             IDefinition def = null;
@@ -97,6 +99,7 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
                 )
                 {
                     omitNew = true;
+                    numericCast = ((IdentifierNode) nameNode).getName().equals(IASLanguageConstants.Number);
                 }
                 
                 if (!((node.getChild(1) instanceof VectorLiteralNode)))
@@ -221,7 +224,10 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
                         && !(NativeUtils.isJSNative(def.getBaseName()))
                         && !def.getBaseName().equals(IASLanguageConstants.XML)
                         && !def.getBaseName().equals(IASLanguageConstants.XMLList);
-                
+
+                if (!isClassCast && def != null && def.getQualifiedName().equals( IASLanguageConstants.Number)){
+                    numericCast = true;
+                }
             }
             
             if (node.isNewExpression())
@@ -244,6 +250,7 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
                             write(JSRoyaleEmitterTokens.UNDERSCORE);
                         write(def.getQualifiedName());
                         endMapping(nameNode);
+                        numericCast = true;
                     } else if( def.getQualifiedName().equals("QName")
                             && getModel().defaultXMLNamespaceActive
                             //is the execution context relevant
@@ -416,6 +423,7 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
                         if (isInt)
                             write(JSRoyaleEmitterTokens.UNDERSCORE);
                         endMapping(node.getNameNode());
+                        numericCast = true;
                     }
                     else if (def.getBaseName().equals("sortOn"))
                 	{
@@ -721,6 +729,26 @@ public class FunctionCallEmitter extends JSSubEmitter implements ISubEmitter<IFu
                     getWalker().walk(node.getNameNode());
                 }
 
+                if (numericCast ) {
+                    if (node.getArgumentsNode().getChildCount() == 1  && SemanticUtils.isXMLish((IExpressionNode) node.getArgumentsNode().getChild(0), getProject())) {
+                        //"patch" the argument node by wrapping in string coercion
+                        ContainerNode args = node.getArgumentsNode();
+                        NodeBase origArg = (NodeBase) args.getChild(0);
+                        args.removeItem(origArg);
+
+                        FunctionCallNode stringCast;
+
+                        if (EmitterUtils.xmlRequiresNullCheck(origArg, getProject())) {
+                            //if it is a simple identifier, then it could be a null reference so use the XMLList.coerce_string method, which retains null
+                            stringCast = EmitterUtils.wrapXMLListStringCoercion(origArg);
+                        } else {
+                            //if it is a member access expression or something else then assume we don't have to check for null
+                            stringCast = EmitterUtils.wrapSimpleStringCoercion(origArg);
+                        }
+
+                        args.addItemAfterNormalization(stringCast);
+                    }
+                }
 
                 getEmitter().emitArguments(node.getArgumentsNode());
     
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/utils/EmitterUtils.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/utils/EmitterUtils.java
index d5a9b0fcc..6f0058460 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/utils/EmitterUtils.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/utils/EmitterUtils.java
@@ -975,4 +975,47 @@ public class EmitterUtils
         return base + "_" + DefinitionUtils.deltaFromObject(definition, project) +"_";
     }
 
+    public static final FunctionCallNode wrapXMLListStringCoercion(NodeBase orig) {
+        //create a call to XMLList.coerce_string(orig)
+        IdentifierNode xmllistClassRef = new IdentifierNode(IASLanguageConstants.XMLList);
+        IdentifierNode castMethod = new IdentifierNode("coerce_string");
+
+        MemberAccessExpressionNode castNode = new MemberAccessExpressionNode(xmllistClassRef,null, castMethod);
+        xmllistClassRef.setParent(castNode);
+        castMethod.setParent(castNode);
+        FunctionCallNode stringCast = new FunctionCallNode(castNode);
+        //explicit parent setting for name and arguments:
+        castNode.setParent(stringCast);
+        stringCast.getArgumentsNode().setParent(stringCast);
+        //wrap the original node
+        stringCast.getArgumentsNode().addItem((NodeBase) orig);
+        return stringCast;
+    }
+
+
+    public static final FunctionCallNode wrapSimpleStringCoercion(NodeBase orig) {
+        //create a call to String(orig)
+        IdentifierNode castNode = new IdentifierNode(IASLanguageConstants.String);
+        FunctionCallNode stringCast = new FunctionCallNode(castNode);
+        //explicit parent setting for name and arguments:
+        castNode.setParent(stringCast);
+        stringCast.getArgumentsNode().setParent(stringCast);
+        //wrap the original node
+        stringCast.getArgumentsNode().addItem((NodeBase) orig);
+        return stringCast;
+    }
+
+    public static final boolean xmlRequiresNullCheck(NodeBase orig, ICompilerProject project) {
+        boolean ret = false;
+        if (orig instanceof IIdentifierNode) {
+            ret = true;
+        } else if (orig instanceof IBinaryOperatorNode) {
+            //should cover IDynamicAccessNode and IMemberAccessExpressionNode
+            //if the left node is xmlish, then we don't need null check
+            ret = !isLeftNodeXMLish(((IBinaryOperatorNode) orig).getLeftOperandNode(),project);
+        }
+
+        return ret;
+    }
+
 }
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 38260fae7..7c40e166e 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
@@ -472,7 +472,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
     {
         IBinaryOperatorNode node = getBinaryNode("var var1:String;var var2:XML;var1 = var2.child");
         asBlockWalker.visitBinaryOperator(node);
-        assertOut("var1 = org.apache.royale.utils.Language.string(var2.child('child'))");
+        assertOut("var1 = String(var2.child('child'))");
     }
 
     @Test
@@ -1976,7 +1976,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
     {
         IReturnNode node = (IReturnNode) getNode("function():String { var a:XML; return a.child; }", IReturnNode.class);
         asBlockWalker.visitReturn(node);
-        assertOut("return org.apache.royale.utils.Language.string(a.child('child'))");
+        assertOut("return String(a.child('child'))");
     }
 
     @Test
@@ -2144,7 +2144,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
     {
         IFunctionCallNode node = (IFunctionCallNode) getNode("function a(foo:String):void {}; var b:XML; a(b.child);", IFunctionCallNode.class);
         asBlockWalker.visitFunctionCall(node);
-        assertOut("a(org.apache.royale.utils.Language.string(b.child('child')))");
+        assertOut("a(String(b.child('child')))");
     }
 
     @Test
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
index d93c34ee6..582b920b9 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
@@ -1188,7 +1188,7 @@ public class TestRoyaleGlobalClasses extends TestGoogGlobalClasses
         IASNode parentNode = node.getParent();
         node = (IVariableNode) parentNode.getChild(1);
         asBlockWalker.visitVariable(node);
-        assertOut("var /** @type {string} */ b = org.apache.royale.utils.Language.string(a.attribute('attr1'))");
+        assertOut("var /** @type {string} */ b = String(a.attribute('attr1'))");
     }
     
     @Test
@@ -1196,7 +1196,7 @@ public class TestRoyaleGlobalClasses extends TestGoogGlobalClasses
     {
     	IBinaryOperatorNode node = (IBinaryOperatorNode)getNode("var a:XML = new XML(\"<top attr1='cat'><child attr2='dog'><grandchild attr3='fish'>text</grandchild></child></top>\");var b:String; b = a.@attr1;", IBinaryOperatorNode.class);
         asBlockWalker.visitBinaryOperator(node);
-        assertOut("b = org.apache.royale.utils.Language.string(a.attribute('attr1'))");
+        assertOut("b = String(a.attribute('attr1'))");
     }
     
     @Test