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 e479497 Fix for issue #118. Also fixes: SWF Bindable codegen for
getter/setter combos now matches original Flex behaviour. Avoids unnecessary
output of extra getters in swf which do nothing useful (extra layer in the
stack that just calls the same original code in renamed private getters).
e479497 is described below
commit e479497c89837e90047153d8ed7361fc761c048e
Author: greg-dove <[email protected]>
AuthorDate: Tue Feb 18 15:55:52 2020 +1300
Fix for issue #118.
Also fixes: SWF Bindable codegen for getter/setter combos now matches
original Flex behaviour. Avoids unnecessary output of extra getters in swf
which do nothing useful (extra layer in the stack that just calls the same
original code in renamed private getters).
---
.../royale/compiler/codegen/js/IJSEmitter.java | 1 +
.../compiler/internal/codegen/js/JSEmitter.java | 8 +++
.../internal/codegen/js/jx/AccessorEmitter.java | 15 +++---
.../internal/codegen/js/jx/StatementEmitter.java | 2 +-
.../codegen/js/royale/TestRoyaleAccessors.java | 27 +++++++---
.../internal/as/codegen/BindableHelper.java | 5 +-
.../as/codegen/ClassDirectiveProcessor.java | 59 +++++++---------------
.../compiler/internal/as/codegen/LexicalScope.java | 14 +++++
8 files changed, 73 insertions(+), 58 deletions(-)
diff --git
a/compiler-jx/src/main/java/org/apache/royale/compiler/codegen/js/IJSEmitter.java
b/compiler-jx/src/main/java/org/apache/royale/compiler/codegen/js/IJSEmitter.java
index b68f9f0..c20261c 100644
---
a/compiler-jx/src/main/java/org/apache/royale/compiler/codegen/js/IJSEmitter.java
+++
b/compiler-jx/src/main/java/org/apache/royale/compiler/codegen/js/IJSEmitter.java
@@ -41,6 +41,7 @@ public interface IJSEmitter extends IASEmitter,
IMappingEmitter
String formatQualifiedName(String name);
String formatPrivateName(String className, String name);
+ String formatPrivateName(String className, String name, Boolean nameFirst);
void emitSourceMapDirective(ITypeNode node);
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 d6a9745..d24adca 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
@@ -534,6 +534,14 @@ public class JSEmitter extends ASEmitter implements
IJSEmitter
return className.replace(".", "_") + "_" + name;
}
+ public String formatPrivateName(String className, String name, Boolean
nameFirst) {
+ if (nameFirst) {
+ return name + "_" +className.replace(".", "_");
+ } else {
+ return formatPrivateName(className, name);
+ }
+ }
+
public void emitAssignmentCoercion(IExpressionNode assignedNode,
IDefinition definition)
{
IDefinition assignedDef = null;
diff --git
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
index 6d18011..2b0ab7c 100644
---
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
+++
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
@@ -45,7 +45,6 @@ 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.JSGoogDocEmitter;
import org.apache.royale.compiler.internal.codegen.js.goog.JSGoogEmitterTokens;
-import org.apache.royale.compiler.internal.definitions.NamespaceDefinition;
import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
import org.apache.royale.compiler.internal.semantics.SemanticUtils;
import org.apache.royale.compiler.internal.tree.as.FunctionNode;
@@ -216,10 +215,14 @@ public class AccessorEmitter extends JSSubEmitter
implements
else
{
write(ASEmitterTokens.MEMBER_ACCESS);
- if (isBindable)
-
write(JSRoyaleEmitterTokens.BINDABLE_PREFIX);
- write(JSRoyaleEmitterTokens.SETTER_PREFIX);
- write(baseName);
+ if (isBindable) {
+
write(JSRoyaleEmitterTokens.BINDABLE_PREFIX);
+
write(JSRoyaleEmitterTokens.SETTER_PREFIX);
+
write(getEmitter().formatPrivateName(definition.getQualifiedName(), baseName,
true));
+ } else {
+
write(JSRoyaleEmitterTokens.SETTER_PREFIX);
+ write(baseName);
+ }
}
write(ASEmitterTokens.SPACE);
write(ASEmitterTokens.EQUAL);
@@ -275,7 +278,7 @@ public class AccessorEmitter extends JSSubEmitter implements
write(ASEmitterTokens.MEMBER_ACCESS);
write(JSRoyaleEmitterTokens.BINDABLE_PREFIX);
write(JSRoyaleEmitterTokens.SETTER_PREFIX);
- write(baseName);
+
write(getEmitter().formatPrivateName(definition.getQualifiedName(), baseName,
true));
write(ASEmitterTokens.PAREN_OPEN);
write("value");
write(ASEmitterTokens.PAREN_CLOSE);
diff --git
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/StatementEmitter.java
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/StatementEmitter.java
index e58208c..912b962 100644
---
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/StatementEmitter.java
+++
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/StatementEmitter.java
@@ -49,7 +49,7 @@ public class StatementEmitter extends JSSubEmitter implements
if (node.getParent().getNodeID() != ASTNodeID.LabledStatementID
&& node.getNodeID() != ASTNodeID.ConfigBlockID
&& !(node instanceof IStatementNode))
- {
+ { //@todo - the following can generate a lonely ";\n" in the js output
from 'super()' in the constructor call
startMapping(node, node);
write(ASEmitterTokens.SEMICOLON);
endMapping(node);
diff --git
a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
index 6330183..fabe07f 100644
---
a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
+++
b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
@@ -162,14 +162,25 @@ public class TestRoyaleAccessors extends ASTestBase
"public function doStuff():void {label = 'hello, bye'; var
theLabel:String = label;}; private var _label:String; [Bindable] public
function get label():String {return _label}; public function set
label(value:String):void {_label = value}; ",
IClassNode.class, WRAP_LEVEL_CLASS);
asBlockWalker.visitClass(node);
- String expected = "/**\n * @constructor\n */\nRoyaleTest_A =
function() {\n};\n\n\n/**\n * Prevent renaming of class. Needed for
reflection.\n */\ngoog.exportSymbol('RoyaleTest_A', RoyaleTest_A);\n\n\n/**\n *
@export\n */\nRoyaleTest_A.prototype.doStuff = function() {\n this.label =
'hello, bye';\n var /** @type {string} */ theLabel =
this.label;\n};\n\n\n/**\n * @private\n * @type {string}\n
*/\nRoyaleTest_A.prototype._label = null;\n\n\n" +
- "RoyaleTest_A.prototype.get__label = function() {\n
return this._label;\n};\n\n\n" +
- "RoyaleTest_A.prototype.bindable__set__label =
function(value) {\n this._label = value;\n};\n\n\n" +
- "RoyaleTest_A.prototype.set__label = function(value)
{\nvar oldValue = this.get__label();\nif (oldValue != value)
{\nthis.bindable__set__label(value);\n" +
- "
this.dispatchEvent(org.apache.royale.events.ValueChangeEvent.createUpdateEvent(\n"
+
- " this, \"label\", oldValue,
value));\n}\n};\n\n\n" +
- "Object.defineProperties(RoyaleTest_A.prototype, /**
@lends {RoyaleTest_A.prototype} */ {\n/**\n * @export\n * @type {string}
*/\n" +
- "label: {\nget:
RoyaleTest_A.prototype.get__label,\nset:
RoyaleTest_A.prototype.set__label}}\n);";
+ String expected ="/**\n * @constructor\n */\nRoyaleTest_A = function()
{\n};\n\n\n/**\n * Prevent renaming of class. Needed for reflection.\n
*/\ngoog.exportSymbol('RoyaleTest_A', RoyaleTest_A);\n\n\n/**\n * @export\n
*/\nRoyaleTest_A.prototype.doStuff = function() {\n this.label = 'hello,
bye';\n var /** @type {string} */ theLabel = this.label;\n};\n\n\n/**\n *
@private\n * @type {string}\n */\nRoyaleTest_A.prototype._label = null;\n\n\n" +
+ "RoyaleTest_A.prototype.get__label = function() {\n return
this._label;\n};\n\n\n" +
+ "RoyaleTest_A.prototype.bindable__set__label_RoyaleTest_A =
function(value) {\n" +
+ " this._label = value;" +
+ "\n};\n\n\n" +
+ "RoyaleTest_A.prototype.set__label = function(value) {\n" +
+ "var oldValue = this.get__label();\n" +
+ "if (oldValue != value) {\n" +
+ "this.bindable__set__label_RoyaleTest_A(value);\n" +
+ "
this.dispatchEvent(org.apache.royale.events.ValueChangeEvent.createUpdateEvent(\n"
+
+ " this, \"label\", oldValue, value));\n" +
+ "}\n};\n\n\n" +
+ "Object.defineProperties(RoyaleTest_A.prototype, /** @lends
{RoyaleTest_A.prototype} */ {\n" +
+ "/**\n" +
+ " * @export\n * @type {string} */\n" +
+ "label: {\n" +
+ "get: RoyaleTest_A.prototype.get__label,\n" +
+ "set: RoyaleTest_A.prototype.set__label}}\n" +
+ ");";
assertOut(expected);
}
diff --git
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/BindableHelper.java
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/BindableHelper.java
index 73626cb..4bb45a7 100644
---
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/BindableHelper.java
+++
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/BindableHelper.java
@@ -220,9 +220,10 @@ public class BindableHelper
mi.setReturnType(NAME_VOID);
InstructionList insns = new InstructionList(32);
- // var oldValue = backingName;
+ // var oldValue = backingName (or propName for an originally defined
getter);
+ Name oldValuePropName = classScope.hasDeclaredVariableName(propName) ?
backingName : propName;
insns.addInstruction(OP_getlocal0);
- insns.addInstruction(OP_getproperty, backingName);
+ insns.addInstruction(OP_getproperty, oldValuePropName);
insns.addInstruction(OP_setlocal2);
// if( oldValue !== newValue )
diff --git
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/ClassDirectiveProcessor.java
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/ClassDirectiveProcessor.java
index 2f71bfb..82cfa05 100644
---
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/ClassDirectiveProcessor.java
+++
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/ClassDirectiveProcessor.java
@@ -53,6 +53,7 @@ import
org.apache.royale.compiler.definitions.metadata.IMetaTag;
import org.apache.royale.compiler.definitions.metadata.IMetaTagAttribute;
import org.apache.royale.compiler.definitions.references.INamespaceReference;
import org.apache.royale.compiler.exceptions.CodegenInterruptedException;
+import org.apache.royale.compiler.internal.definitions.*;
import org.apache.royale.compiler.internal.tree.as.*;
import org.apache.royale.compiler.problems.*;
import org.apache.royale.compiler.projects.ICompilerProject;
@@ -62,15 +63,6 @@ import org.apache.royale.compiler.tree.as.*;
import
org.apache.royale.compiler.tree.as.ILanguageIdentifierNode.LanguageIdentifierKind;
import org.apache.royale.compiler.internal.abc.FunctionGeneratorHelper;
import
org.apache.royale.compiler.internal.as.codegen.ICodeGenerator.IConstantValue;
-import org.apache.royale.compiler.internal.definitions.AccessorDefinition;
-import org.apache.royale.compiler.internal.definitions.ClassDefinition;
-import org.apache.royale.compiler.internal.definitions.DefinitionBase;
-import org.apache.royale.compiler.internal.definitions.FunctionDefinition;
-import org.apache.royale.compiler.internal.definitions.GetterDefinition;
-import org.apache.royale.compiler.internal.definitions.InterfaceDefinition;
-import org.apache.royale.compiler.internal.definitions.NamespaceDefinition;
-import org.apache.royale.compiler.internal.definitions.TypeDefinitionBase;
-import org.apache.royale.compiler.internal.definitions.VariableDefinition;
import org.apache.royale.compiler.internal.definitions.metadata.MetaTag;
import
org.apache.royale.compiler.internal.definitions.metadata.ResourceBundleMetaTag;
import org.apache.royale.compiler.internal.embedding.EmbedData;
@@ -853,6 +845,8 @@ class ClassDirectiveProcessor extends DirectiveProcessor
Name funcName = funcDef.getMName(classScope.getProject());
Name bindableName = null;
boolean wasOverride = false;
+ //we only mutate the setter definitions, leave getters as-is
+ isBindable = isBindable && funcDef instanceof ISetterDefinition;
if (isBindable)
{
// move function into bindable namespace
@@ -904,41 +898,24 @@ class ClassDirectiveProcessor extends DirectiveProcessor
}
}
if (isBindable)
- {
+ { //only setters get modified in this case, getters remain the same
as source code
if (wasOverride)
funcDef.setOverride();
- if (funcDef instanceof GetterDefinition)
- {
- Name funcTypeName;
- TypeDefinitionBase typeDef = funcDef.resolveType(project);
- if ( SemanticUtils.isType(typeDef) )
- funcTypeName = typeDef.getMName(project);
- else
- funcTypeName = NAME_OBJECT;
- DefinitionBase bindableGetter =
func.buildBindableGetter(funcName.getBaseName());
- ASScope funcScope = (ASScope)funcDef.getContainingScope();
- bindableGetter.setContainingScope(funcScope);
- LexicalScope ls = funcDef.isStatic()? classStaticScope:
classScope;
- ls.generateBindableGetter(bindableGetter, funcName,
bindableName,
- funcTypeName, getAllMetaTags(funcDef));
- }
+
+ Name funcTypeName;
+ TypeDefinitionBase typeDef = funcDef.resolveType(project);
+ if ( SemanticUtils.isType(typeDef) )
+ funcTypeName = typeDef.getMName(project);
else
- {
- Name funcTypeName;
- TypeDefinitionBase typeDef = funcDef.resolveType(project);
- if ( SemanticUtils.isType(typeDef) )
- funcTypeName = typeDef.getMName(project);
- else
- funcTypeName = NAME_OBJECT;
- ASScope funcScope = (ASScope)funcDef.getContainingScope();
- DefinitionBase bindableSetter =
func.buildBindableSetter(funcName.getBaseName(),
- funcScope,
- funcDef.getTypeReference());
- bindableSetter.setContainingScope(funcScope);
- LexicalScope ls = funcDef.isStatic()? classStaticScope:
classScope;
- ls.generateBindableSetter(bindableSetter, funcName,
bindableName,
- funcTypeName, getAllMetaTags(funcDef));
- }
+ funcTypeName = NAME_OBJECT;
+ ASScope funcScope = (ASScope)funcDef.getContainingScope();
+ DefinitionBase bindableSetter =
func.buildBindableSetter(funcName.getBaseName(),
+ funcScope,
+ funcDef.getTypeReference());
+ bindableSetter.setContainingScope(funcScope);
+ LexicalScope ls = funcDef.isStatic()? classStaticScope: classScope;
+ ls.generateBindableSetter(bindableSetter, funcName, bindableName,
+ funcTypeName, getAllMetaTags(funcDef));
}
}
diff --git
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/LexicalScope.java
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/LexicalScope.java
index 8cc3c1f..002661e 100644
---
a/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/LexicalScope.java
+++
b/compiler/src/main/java/org/apache/royale/compiler/internal/as/codegen/LexicalScope.java
@@ -473,6 +473,20 @@ public class LexicalScope
public enum VariableMutability { Default, Variable, Constant };
/**
+ * Check to see if this name is declared as a variable within this scope
+ * @param var_name A Name instance to check
+ * @return true if the var_name is a declared variable.
+ */
+ public boolean hasDeclaredVariableName(Name var_name) {
+ if ( var_name != null && this.declaredVariables.contains(var_name) )
+ {
+ return true;
+ }
+ //consider: do we also need to check non-reference equality?
+ return false;
+ }
+
+ /**
* Create a variable with potential visibility
* to nested scopes.
*
