You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@royale.apache.org by jo...@apache.org on 2021/01/05 23:39:48 UTC
[royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
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
The following commit(s) were added to refs/heads/develop by this push:
new 084a639 AccessorEmitter: don't redeclare instance accessor as variable if it is an override
084a639 is described below
commit 084a639759757024ea95db74b63ddaead33ee9d0
Author: Josh Tynjala <jo...@apache.org>
AuthorDate: Tue Jan 5 15:39:36 2021 -0800
AccessorEmitter: don't redeclare instance accessor as variable if it is an override
Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
---
.../internal/codegen/js/jx/AccessorEmitter.java | 71 +++++++++++-----------
.../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
.../codegen/js/royale/TestRoyaleClass.java | 1 -
.../resources/royale/projects/super/Base_result.js | 8 ---
4 files changed, 41 insertions(+), 51 deletions(-)
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 771a56c..2c193bc 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
@@ -128,42 +128,45 @@ public class AccessorEmitter extends JSSubEmitter implements
}
else
{
- // start by writing out the instance accessors as regular variables
- // because Closure Compiler doesn't properly analyze calls to
- // defineProperties() alone.
- // since there's no analysis, Closure assumes that getters/setters
- // have no side effects, which results in important get/set calls
- // being removed as dead code.
- // defining the accessors as variables first convinces Closure to
- // handle them more intelligently while not preventing them from
- // being real accessors.
- // Source: https://developers.google.com/closure/compiler/docs/limitations
- writeNewline();
- writeNewline();
- writeNewline();
- writeNewline("/**");
- if (p.preventRename)
- writeNewline(" * @nocollapse");
- if (p.resolvedExport && !p.suppressExport)
- writeNewline(" * @export");
- if (p.type != null)
- writeNewline(" * @type {" + JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(), p.type.getPackageName()) + "}");
- writeNewline(" */");
- write(getEmitter().formatQualifiedName(qname));
- write(ASEmitterTokens.MEMBER_ACCESS);
- write(JSEmitterTokens.PROTOTYPE);
- write(ASEmitterTokens.MEMBER_ACCESS);
- if (p.uri != null)
+ IAccessorNode accessorNode = (getterNode != null) ? getterNode : setterNode;
+ if(!accessorNode.getDefinition().isOverride())
{
- IAccessorNode node = (getterNode != null) ? getterNode : setterNode;
- INamespaceDecorationNode ns = ((FunctionNode)node).getActualNamespaceNode();
- INamespaceDefinition nsDef = (INamespaceDefinition)ns.resolve(project);
- fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used names
- write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
+ // start by writing out the instance accessors as regular variables
+ // because Closure Compiler doesn't properly analyze calls to
+ // defineProperties() alone.
+ // since there's no analysis, Closure assumes that getters/setters
+ // have no side effects, which results in important get/set calls
+ // being removed as dead code.
+ // defining the accessors as variables first convinces Closure to
+ // handle them more intelligently while not preventing them from
+ // being real accessors.
+ // Source: https://developers.google.com/closure/compiler/docs/limitations
+ writeNewline();
+ writeNewline();
+ writeNewline();
+ writeNewline("/**");
+ if (p.preventRename)
+ writeNewline(" * @nocollapse");
+ if (p.resolvedExport && !p.suppressExport)
+ writeNewline(" * @export");
+ if (p.type != null)
+ writeNewline(" * @type {" + JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(), p.type.getPackageName()) + "}");
+ writeNewline(" */");
+ write(getEmitter().formatQualifiedName(qname));
+ write(ASEmitterTokens.MEMBER_ACCESS);
+ write(JSEmitterTokens.PROTOTYPE);
+ write(ASEmitterTokens.MEMBER_ACCESS);
+ if (p.uri != null)
+ {
+ INamespaceDecorationNode ns = ((FunctionNode) accessorNode).getActualNamespaceNode();
+ INamespaceDefinition nsDef = (INamespaceDefinition)ns.resolve(project);
+ fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used names
+ write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
+ }
+ else
+ write(baseName);
+ write(ASEmitterTokens.SEMICOLON);
}
- else
- write(baseName);
- write(ASEmitterTokens.SEMICOLON);
if (getterNode != null)
{
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
index c8d4c3a..958902d 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
@@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
IClassNode.class, WRAP_LEVEL_PACKAGE);
asBlockWalker.visitClass(node);
assertOut("/**\n * @constructor\n * @extends {A}\n */\nB = function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B, A);\n\n\n" +
- "/**\n * @nocollapse\n * @export\n * @type {number}\n */\nB.prototype.foo;\n\n\n" +
- "B.prototype.get__foo = function() {\n return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
+ "B.prototype.get__foo = function() {\n return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
"Object.defineProperties(B.prototype, /** @lends {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget: B.prototype.get__foo}}\n);");
}
@@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
IClassNode.class, WRAP_LEVEL_PACKAGE);
asBlockWalker.visitClass(node);
assertOut("/**\n * @constructor\n * @extends {A}\n */\nB = function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B, A);\n\n\n" +
- "/**\n * @nocollapse\n * @export\n * @type {number}\n */\nB.prototype.foo;\n\n\n" +
- "B.prototype.get__foo = function() {\n return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
+ "B.prototype.get__foo = function() {\n return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
"Object.defineProperties(B.prototype, /** @lends {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget: B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
}
@@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
IClassNode.class, WRAP_LEVEL_PACKAGE);
asBlockWalker.visitClass(node);
assertOut("/**\n * @constructor\n * @extends {A}\n */\nB = function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B, A);\n\n\n" +
- "/**\n * @nocollapse\n * @export\n * @type {number}\n */\nB.prototype.foo;\n\n\n" +
- "B.prototype.set__foo = function(value) {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
+ "B.prototype.set__foo = function(value) {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
"Object.defineProperties(B.prototype, /** @lends {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset: B.prototype.set__foo}}\n);");
}
@@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
IClassNode.class, WRAP_LEVEL_PACKAGE);
asBlockWalker.visitClass(node);
assertOut("/**\n * @constructor\n * @extends {A}\n */\nB = function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B, A);\n\n\n" +
- "/**\n * @nocollapse\n * @export\n * @type {number}\n */\nB.prototype.foo;\n\n\n" +
- "B.prototype.set__foo = function(value) {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
+ "B.prototype.set__foo = function(value) {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
"Object.defineProperties(B.prototype, /** @lends {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget: A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
}
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
index a30202b..595dbdd 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
@@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
IClassNode node = getClassNode("public class B extends A {public function B() {}; override public function set foo(value:Object):void {super.foo = value;};} class A {public function set foo(value:Object):void {}}");
asBlockWalker.visitClass(node);
String expected = "/**\n * @constructor\n * @extends {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n org.apache.royale.B.base(this, 'constructor');\n};\ngoog.inherits(org.apache.royale.B, org.apache.royale.A);\n\n\n" +
- "/**\n * @nocollapse\n * @export\n * @type {Object}\n */\norg.apache.royale.B.prototype.foo;\n\n\n" +
"org.apache.royale.B.prototype.set__foo = function(value) {\n org.apache.royale.B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
"Object.defineProperties(org.apache.royale.B.prototype, /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type {Object}\n */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
assertOut(expected);
diff --git a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
index 169067a..fcaa48c 100644
--- a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
+++ b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
@@ -35,14 +35,6 @@ Base = function() {
goog.inherits(Base, Super);
-/**
- * @nocollapse
- * @export
- * @type {string}
- */
-Base.prototype.text;
-
-
Base.prototype.get__text = function() {
return "A" + Base.superClass_.get__text.apply(this);
};
Re: [royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
Posted by Greg Dove <gr...@gmail.com>.
Seems it was that, thanks again for the prompt, Josh. I was able to revert
most of the changes and still add something minimalistic to fix the
original issue it was addressing.
On Thu, Jan 7, 2021 at 7:37 AM Greg Dove <gr...@gmail.com> wrote:
>
> Thanks Josh, that's a good point, I will check that later today.
>
> On Thu, Jan 7, 2021 at 6:29 AM Josh Tynjala <jo...@bowlerhat.dev>
> wrote:
>
>> Hi Greg,
>>
>> I don't think that my changes would affect the dependency lists, but I am
>> only somewhat familiar with how those are generated. It's certainly
>> possible, but nothing stands out as obvious to me.
>>
>> Changes to how static initializer code is generated could potentially
>> affect the dependency lists. Your recent commit in that area might be
>> worth
>> double-checking.
>>
>> --
>> Josh Tynjala
>> Bowler Hat LLC <https://bowlerhat.dev>
>>
>>
>> On Tue, Jan 5, 2021 at 4:36 PM Greg Dove <gr...@gmail.com> wrote:
>>
>> > Hi Josh,
>> >
>> > That seems to fix that specific issue. But fyi I am seeing some
>> differences
>> > in goog.require output in some cases between 0.9.8-SNAPSHOT and
>> > 0.9.9-SNAPSHOT compiler build that might be what breaks things at
>> runtime
>> > in the project we are working on. Are these (or other recent changes)
>> > supposed to change the goog.require part of the output?
>> >
>> > I see changes in the commented out 'Royale Dependency List' and 'Royale
>> > Static Dependency List' (the latest compiler code seems to have what
>> were
>> > previously 'static' dependencies moved to )
>> >
>> > And I see items that were in the goog.requires missing in cases where
>> they
>> > were previously included.
>> >
>> > I can't provide more details now, sorry. I will try to circle back to
>> this
>> > later in the week if it is not obvious to you.
>> >
>> >
>> > On Wed, Jan 6, 2021 at 12:46 PM Greg Dove <gr...@gmail.com> wrote:
>> >
>> > > Oops, you already got there! Please ignore my previous comment :)
>> > >
>> > >
>> > > On Wed, Jan 6, 2021 at 12:39 PM <jo...@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
>> > >>
>> > >>
>> > >> The following commit(s) were added to refs/heads/develop by this
>> push:
>> > >> new 084a639 AccessorEmitter: don't redeclare instance accessor
>> as
>> > >> variable if it is an override
>> > >> 084a639 is described below
>> > >>
>> > >> commit 084a639759757024ea95db74b63ddaead33ee9d0
>> > >> Author: Josh Tynjala <jo...@apache.org>
>> > >> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
>> > >>
>> > >> AccessorEmitter: don't redeclare instance accessor as variable
>> if it
>> > >> is an override
>> > >>
>> > >> Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
>> > >> ---
>> > >> .../internal/codegen/js/jx/AccessorEmitter.java | 71
>> > >> +++++++++++-----------
>> > >> .../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
>> > >> .../codegen/js/royale/TestRoyaleClass.java | 1 -
>> > >> .../resources/royale/projects/super/Base_result.js | 8 ---
>> > >> 4 files changed, 41 insertions(+), 51 deletions(-)
>> > >>
>> > >> 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 771a56c..2c193bc 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
>> > >> @@ -128,42 +128,45 @@ public class AccessorEmitter extends
>> JSSubEmitter
>> > >> implements
>> > >> }
>> > >> else
>> > >> {
>> > >> - // start by writing out the instance accessors
>> as
>> > >> regular variables
>> > >> - // because Closure Compiler doesn't properly
>> > analyze
>> > >> calls to
>> > >> - // defineProperties() alone.
>> > >> - // since there's no analysis, Closure assumes
>> that
>> > >> getters/setters
>> > >> - // have no side effects, which results in
>> important
>> > >> get/set calls
>> > >> - // being removed as dead code.
>> > >> - // defining the accessors as variables first
>> > >> convinces Closure to
>> > >> - // handle them more intelligently while not
>> > >> preventing them from
>> > >> - // being real accessors.
>> > >> - // Source:
>> > >> https://developers.google.com/closure/compiler/docs/limitations
>> > >> - writeNewline();
>> > >> - writeNewline();
>> > >> - writeNewline();
>> > >> - writeNewline("/**");
>> > >> - if (p.preventRename)
>> > >> - writeNewline(" * @nocollapse");
>> > >> - if (p.resolvedExport && !p.suppressExport)
>> > >> - writeNewline(" * @export");
>> > >> - if (p.type != null)
>> > >> - writeNewline(" * @type {" +
>> > >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
>> > >> p.type.getPackageName()) + "}");
>> > >> - writeNewline(" */");
>> > >> - write(getEmitter().formatQualifiedName(qname));
>> > >> - write(ASEmitterTokens.MEMBER_ACCESS);
>> > >> - write(JSEmitterTokens.PROTOTYPE);
>> > >> - write(ASEmitterTokens.MEMBER_ACCESS);
>> > >> - if (p.uri != null)
>> > >> + IAccessorNode accessorNode = (getterNode !=
>> null) ?
>> > >> getterNode : setterNode;
>> > >> + if(!accessorNode.getDefinition().isOverride())
>> > >> {
>> > >> - IAccessorNode node = (getterNode != null) ?
>> > >> getterNode : setterNode;
>> > >> - INamespaceDecorationNode ns =
>> > >> ((FunctionNode)node).getActualNamespaceNode();
>> > >> - INamespaceDefinition nsDef =
>> > >> (INamespaceDefinition)ns.resolve(project);
>> > >> -
>> > >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with
>> used
>> > >> names
>> > >> -
>> > >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName,
>> false));
>> > >> + // start by writing out the instance
>> accessors
>> > >> as regular variables
>> > >> + // because Closure Compiler doesn't properly
>> > >> analyze calls to
>> > >> + // defineProperties() alone.
>> > >> + // since there's no analysis, Closure
>> assumes
>> > >> that getters/setters
>> > >> + // have no side effects, which results in
>> > >> important get/set calls
>> > >> + // being removed as dead code.
>> > >> + // defining the accessors as variables first
>> > >> convinces Closure to
>> > >> + // handle them more intelligently while not
>> > >> preventing them from
>> > >> + // being real accessors.
>> > >> + // Source:
>> > >> https://developers.google.com/closure/compiler/docs/limitations
>> > >> + writeNewline();
>> > >> + writeNewline();
>> > >> + writeNewline();
>> > >> + writeNewline("/**");
>> > >> + if (p.preventRename)
>> > >> + writeNewline(" * @nocollapse");
>> > >> + if (p.resolvedExport && !p.suppressExport)
>> > >> + writeNewline(" * @export");
>> > >> + if (p.type != null)
>> > >> + writeNewline(" * @type {" +
>> > >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
>> > >> p.type.getPackageName()) + "}");
>> > >> + writeNewline(" */");
>> > >> +
>> write(getEmitter().formatQualifiedName(qname));
>> > >> + write(ASEmitterTokens.MEMBER_ACCESS);
>> > >> + write(JSEmitterTokens.PROTOTYPE);
>> > >> + write(ASEmitterTokens.MEMBER_ACCESS);
>> > >> + if (p.uri != null)
>> > >> + {
>> > >> + INamespaceDecorationNode ns =
>> > >> ((FunctionNode) accessorNode).getActualNamespaceNode();
>> > >> + INamespaceDefinition nsDef =
>> > >> (INamespaceDefinition)ns.resolve(project);
>> > >> +
>> > >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with
>> used
>> > >> names
>> > >> +
>> > >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName,
>> false));
>> > >> + }
>> > >> + else
>> > >> + write(baseName);
>> > >> + write(ASEmitterTokens.SEMICOLON);
>> > >> }
>> > >> - else
>> > >> - write(baseName);
>> > >> - write(ASEmitterTokens.SEMICOLON);
>> > >>
>> > >> if (getterNode != null)
>> > >> {
>> > >> diff --git
>> > >>
>> >
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> > >>
>> >
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> > >> index c8d4c3a..958902d 100644
>> > >> ---
>> > >>
>> >
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> > >> +++
>> > >>
>> >
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> > >> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
>> > >> TestGoogAccessorMembers
>> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> > >> asBlockWalker.visitClass(node);
>> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> > >> A);\n\n\n" +
>> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> > >> */\nB.prototype.foo;\n\n\n" +
>> > >> - "B.prototype.get__foo = function()
>> {\n
>> > >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> > >> + "B.prototype.get__foo = function() {\n return
>> > >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> > >> "Object.defineProperties(B.prototype, /**
>> @lends
>> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> > >> B.prototype.get__foo}}\n);");
>> > >> }
>> > >>
>> > >> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
>> > >> TestGoogAccessorMembers
>> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> > >> asBlockWalker.visitClass(node);
>> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> > >> A);\n\n\n" +
>> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> > >> */\nB.prototype.foo;\n\n\n" +
>> > >> - "B.prototype.get__foo = function()
>> {\n
>> > >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> > >> + "B.prototype.get__foo = function() {\n return
>> > >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> > >> "Object.defineProperties(B.prototype, /**
>> @lends
>> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> > >> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
>> > >> }
>> > >>
>> > >> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
>> > >> TestGoogAccessorMembers
>> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> > >> asBlockWalker.visitClass(node);
>> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> > >> A);\n\n\n" +
>> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> > >> */\nB.prototype.foo;\n\n\n" +
>> > >> - "B.prototype.set__foo =
>> function(value)
>> > >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> > >> + "B.prototype.set__foo = function(value) {\n
>> > >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> > >> "Object.defineProperties(B.prototype, /**
>> @lends
>> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
>> > >> B.prototype.set__foo}}\n);");
>> > >> }
>> > >>
>> > >> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
>> > >> TestGoogAccessorMembers
>> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> > >> asBlockWalker.visitClass(node);
>> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> > >> A);\n\n\n" +
>> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> > >> */\nB.prototype.foo;\n\n\n" +
>> > >> - "B.prototype.set__foo =
>> function(value)
>> > >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> > >> + "B.prototype.set__foo = function(value) {\n
>> > >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> > >> "Object.defineProperties(B.prototype, /**
>> @lends
>> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> > >> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
>> > >> }
>> > >>
>> > >> diff --git
>> > >>
>> >
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> > >>
>> >
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> > >> index a30202b..595dbdd 100644
>> > >> ---
>> > >>
>> >
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> > >> +++
>> > >>
>> >
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> > >> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends
>> TestGoogClass
>> > >> IClassNode node = getClassNode("public class B extends A
>> > {public
>> > >> function B() {}; override public function set foo(value:Object):void
>> > >> {super.foo = value;};} class A {public function set
>> > foo(value:Object):void
>> > >> {}}");
>> > >> asBlockWalker.visitClass(node);
>> > >> String expected = "/**\n * @constructor\n * @extends
>> > >> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
>> > >> org.apache.royale.B.base(this,
>> > >> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
>> > >> org.apache.royale.A);\n\n\n" +
>> > >> - "/**\n * @nocollapse\n * @export\n * @type
>> {Object}\n
>> > >> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
>> > >> "org.apache.royale.B.prototype.set__foo =
>> > >> function(value) {\n
>> > org.apache.royale.B.superClass_.set__foo.apply(this, [
>> > >> value] );\n};\n\n\n" +
>> > >>
>> "Object.defineProperties(org.apache.royale.B.prototype,
>> > >> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type
>> > {Object}\n
>> > >> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
>> > >> assertOut(expected);
>> > >> diff --git
>> > >> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> > >> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> > >> index 169067a..fcaa48c 100644
>> > >> ---
>> > a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> > >> +++
>> > b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> > >> @@ -35,14 +35,6 @@ Base = function() {
>> > >> goog.inherits(Base, Super);
>> > >>
>> > >>
>> > >> -/**
>> > >> - * @nocollapse
>> > >> - * @export
>> > >> - * @type {string}
>> > >> - */
>> > >> -Base.prototype.text;
>> > >> -
>> > >> -
>> > >> Base.prototype.get__text = function() {
>> > >> return "A" + Base.superClass_.get__text.apply(this);
>> > >> };
>> > >>
>> > >>
>> >
>>
>
Re: [royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
Posted by Greg Dove <gr...@gmail.com>.
Thanks Josh, that's a good point, I will check that later today.
On Thu, Jan 7, 2021 at 6:29 AM Josh Tynjala <jo...@bowlerhat.dev>
wrote:
> Hi Greg,
>
> I don't think that my changes would affect the dependency lists, but I am
> only somewhat familiar with how those are generated. It's certainly
> possible, but nothing stands out as obvious to me.
>
> Changes to how static initializer code is generated could potentially
> affect the dependency lists. Your recent commit in that area might be worth
> double-checking.
>
> --
> Josh Tynjala
> Bowler Hat LLC <https://bowlerhat.dev>
>
>
> On Tue, Jan 5, 2021 at 4:36 PM Greg Dove <gr...@gmail.com> wrote:
>
> > Hi Josh,
> >
> > That seems to fix that specific issue. But fyi I am seeing some
> differences
> > in goog.require output in some cases between 0.9.8-SNAPSHOT and
> > 0.9.9-SNAPSHOT compiler build that might be what breaks things at runtime
> > in the project we are working on. Are these (or other recent changes)
> > supposed to change the goog.require part of the output?
> >
> > I see changes in the commented out 'Royale Dependency List' and 'Royale
> > Static Dependency List' (the latest compiler code seems to have what were
> > previously 'static' dependencies moved to )
> >
> > And I see items that were in the goog.requires missing in cases where
> they
> > were previously included.
> >
> > I can't provide more details now, sorry. I will try to circle back to
> this
> > later in the week if it is not obvious to you.
> >
> >
> > On Wed, Jan 6, 2021 at 12:46 PM Greg Dove <gr...@gmail.com> wrote:
> >
> > > Oops, you already got there! Please ignore my previous comment :)
> > >
> > >
> > > On Wed, Jan 6, 2021 at 12:39 PM <jo...@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
> > >>
> > >>
> > >> The following commit(s) were added to refs/heads/develop by this push:
> > >> new 084a639 AccessorEmitter: don't redeclare instance accessor
> as
> > >> variable if it is an override
> > >> 084a639 is described below
> > >>
> > >> commit 084a639759757024ea95db74b63ddaead33ee9d0
> > >> Author: Josh Tynjala <jo...@apache.org>
> > >> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
> > >>
> > >> AccessorEmitter: don't redeclare instance accessor as variable if
> it
> > >> is an override
> > >>
> > >> Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
> > >> ---
> > >> .../internal/codegen/js/jx/AccessorEmitter.java | 71
> > >> +++++++++++-----------
> > >> .../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
> > >> .../codegen/js/royale/TestRoyaleClass.java | 1 -
> > >> .../resources/royale/projects/super/Base_result.js | 8 ---
> > >> 4 files changed, 41 insertions(+), 51 deletions(-)
> > >>
> > >> 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 771a56c..2c193bc 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
> > >> @@ -128,42 +128,45 @@ public class AccessorEmitter extends
> JSSubEmitter
> > >> implements
> > >> }
> > >> else
> > >> {
> > >> - // start by writing out the instance accessors as
> > >> regular variables
> > >> - // because Closure Compiler doesn't properly
> > analyze
> > >> calls to
> > >> - // defineProperties() alone.
> > >> - // since there's no analysis, Closure assumes
> that
> > >> getters/setters
> > >> - // have no side effects, which results in
> important
> > >> get/set calls
> > >> - // being removed as dead code.
> > >> - // defining the accessors as variables first
> > >> convinces Closure to
> > >> - // handle them more intelligently while not
> > >> preventing them from
> > >> - // being real accessors.
> > >> - // Source:
> > >> https://developers.google.com/closure/compiler/docs/limitations
> > >> - writeNewline();
> > >> - writeNewline();
> > >> - writeNewline();
> > >> - writeNewline("/**");
> > >> - if (p.preventRename)
> > >> - writeNewline(" * @nocollapse");
> > >> - if (p.resolvedExport && !p.suppressExport)
> > >> - writeNewline(" * @export");
> > >> - if (p.type != null)
> > >> - writeNewline(" * @type {" +
> > >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> > >> p.type.getPackageName()) + "}");
> > >> - writeNewline(" */");
> > >> - write(getEmitter().formatQualifiedName(qname));
> > >> - write(ASEmitterTokens.MEMBER_ACCESS);
> > >> - write(JSEmitterTokens.PROTOTYPE);
> > >> - write(ASEmitterTokens.MEMBER_ACCESS);
> > >> - if (p.uri != null)
> > >> + IAccessorNode accessorNode = (getterNode !=
> null) ?
> > >> getterNode : setterNode;
> > >> + if(!accessorNode.getDefinition().isOverride())
> > >> {
> > >> - IAccessorNode node = (getterNode != null) ?
> > >> getterNode : setterNode;
> > >> - INamespaceDecorationNode ns =
> > >> ((FunctionNode)node).getActualNamespaceNode();
> > >> - INamespaceDefinition nsDef =
> > >> (INamespaceDefinition)ns.resolve(project);
> > >> -
> > >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with
> used
> > >> names
> > >> -
> > >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName,
> false));
> > >> + // start by writing out the instance
> accessors
> > >> as regular variables
> > >> + // because Closure Compiler doesn't properly
> > >> analyze calls to
> > >> + // defineProperties() alone.
> > >> + // since there's no analysis, Closure assumes
> > >> that getters/setters
> > >> + // have no side effects, which results in
> > >> important get/set calls
> > >> + // being removed as dead code.
> > >> + // defining the accessors as variables first
> > >> convinces Closure to
> > >> + // handle them more intelligently while not
> > >> preventing them from
> > >> + // being real accessors.
> > >> + // Source:
> > >> https://developers.google.com/closure/compiler/docs/limitations
> > >> + writeNewline();
> > >> + writeNewline();
> > >> + writeNewline();
> > >> + writeNewline("/**");
> > >> + if (p.preventRename)
> > >> + writeNewline(" * @nocollapse");
> > >> + if (p.resolvedExport && !p.suppressExport)
> > >> + writeNewline(" * @export");
> > >> + if (p.type != null)
> > >> + writeNewline(" * @type {" +
> > >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> > >> p.type.getPackageName()) + "}");
> > >> + writeNewline(" */");
> > >> +
> write(getEmitter().formatQualifiedName(qname));
> > >> + write(ASEmitterTokens.MEMBER_ACCESS);
> > >> + write(JSEmitterTokens.PROTOTYPE);
> > >> + write(ASEmitterTokens.MEMBER_ACCESS);
> > >> + if (p.uri != null)
> > >> + {
> > >> + INamespaceDecorationNode ns =
> > >> ((FunctionNode) accessorNode).getActualNamespaceNode();
> > >> + INamespaceDefinition nsDef =
> > >> (INamespaceDefinition)ns.resolve(project);
> > >> +
> > >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with
> used
> > >> names
> > >> +
> > >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName,
> false));
> > >> + }
> > >> + else
> > >> + write(baseName);
> > >> + write(ASEmitterTokens.SEMICOLON);
> > >> }
> > >> - else
> > >> - write(baseName);
> > >> - write(ASEmitterTokens.SEMICOLON);
> > >>
> > >> if (getterNode != null)
> > >> {
> > >> diff --git
> > >>
> >
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> > >>
> >
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> > >> index c8d4c3a..958902d 100644
> > >> ---
> > >>
> >
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> > >> +++
> > >>
> >
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> > >> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
> > >> TestGoogAccessorMembers
> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> > >> asBlockWalker.visitClass(node);
> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> > >> A);\n\n\n" +
> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> > >> */\nB.prototype.foo;\n\n\n" +
> > >> - "B.prototype.get__foo = function() {\n
> > >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> > >> + "B.prototype.get__foo = function() {\n return
> > >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> > >> "Object.defineProperties(B.prototype, /**
> @lends
> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> > >> B.prototype.get__foo}}\n);");
> > >> }
> > >>
> > >> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
> > >> TestGoogAccessorMembers
> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> > >> asBlockWalker.visitClass(node);
> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> > >> A);\n\n\n" +
> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> > >> */\nB.prototype.foo;\n\n\n" +
> > >> - "B.prototype.get__foo = function() {\n
> > >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> > >> + "B.prototype.get__foo = function() {\n return
> > >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> > >> "Object.defineProperties(B.prototype, /**
> @lends
> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> > >> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
> > >> }
> > >>
> > >> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
> > >> TestGoogAccessorMembers
> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> > >> asBlockWalker.visitClass(node);
> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> > >> A);\n\n\n" +
> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> > >> */\nB.prototype.foo;\n\n\n" +
> > >> - "B.prototype.set__foo =
> function(value)
> > >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> > >> + "B.prototype.set__foo = function(value) {\n
> > >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> > >> "Object.defineProperties(B.prototype, /**
> @lends
> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
> > >> B.prototype.set__foo}}\n);");
> > >> }
> > >>
> > >> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
> > >> TestGoogAccessorMembers
> > >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> > >> asBlockWalker.visitClass(node);
> > >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> > >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> > >> A);\n\n\n" +
> > >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> > >> */\nB.prototype.foo;\n\n\n" +
> > >> - "B.prototype.set__foo =
> function(value)
> > >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> > >> + "B.prototype.set__foo = function(value) {\n
> > >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> > >> "Object.defineProperties(B.prototype, /**
> @lends
> > >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> > >> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
> > >> }
> > >>
> > >> diff --git
> > >>
> >
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> > >>
> >
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> > >> index a30202b..595dbdd 100644
> > >> ---
> > >>
> >
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> > >> +++
> > >>
> >
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> > >> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
> > >> IClassNode node = getClassNode("public class B extends A
> > {public
> > >> function B() {}; override public function set foo(value:Object):void
> > >> {super.foo = value;};} class A {public function set
> > foo(value:Object):void
> > >> {}}");
> > >> asBlockWalker.visitClass(node);
> > >> String expected = "/**\n * @constructor\n * @extends
> > >> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
> > >> org.apache.royale.B.base(this,
> > >> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
> > >> org.apache.royale.A);\n\n\n" +
> > >> - "/**\n * @nocollapse\n * @export\n * @type {Object}\n
> > >> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
> > >> "org.apache.royale.B.prototype.set__foo =
> > >> function(value) {\n
> > org.apache.royale.B.superClass_.set__foo.apply(this, [
> > >> value] );\n};\n\n\n" +
> > >>
> "Object.defineProperties(org.apache.royale.B.prototype,
> > >> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type
> > {Object}\n
> > >> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
> > >> assertOut(expected);
> > >> diff --git
> > >> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> > >> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> > >> index 169067a..fcaa48c 100644
> > >> ---
> > a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> > >> +++
> > b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> > >> @@ -35,14 +35,6 @@ Base = function() {
> > >> goog.inherits(Base, Super);
> > >>
> > >>
> > >> -/**
> > >> - * @nocollapse
> > >> - * @export
> > >> - * @type {string}
> > >> - */
> > >> -Base.prototype.text;
> > >> -
> > >> -
> > >> Base.prototype.get__text = function() {
> > >> return "A" + Base.superClass_.get__text.apply(this);
> > >> };
> > >>
> > >>
> >
>
Re: [royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
Posted by Josh Tynjala <jo...@bowlerhat.dev>.
Hi Greg,
I don't think that my changes would affect the dependency lists, but I am
only somewhat familiar with how those are generated. It's certainly
possible, but nothing stands out as obvious to me.
Changes to how static initializer code is generated could potentially
affect the dependency lists. Your recent commit in that area might be worth
double-checking.
--
Josh Tynjala
Bowler Hat LLC <https://bowlerhat.dev>
On Tue, Jan 5, 2021 at 4:36 PM Greg Dove <gr...@gmail.com> wrote:
> Hi Josh,
>
> That seems to fix that specific issue. But fyi I am seeing some differences
> in goog.require output in some cases between 0.9.8-SNAPSHOT and
> 0.9.9-SNAPSHOT compiler build that might be what breaks things at runtime
> in the project we are working on. Are these (or other recent changes)
> supposed to change the goog.require part of the output?
>
> I see changes in the commented out 'Royale Dependency List' and 'Royale
> Static Dependency List' (the latest compiler code seems to have what were
> previously 'static' dependencies moved to )
>
> And I see items that were in the goog.requires missing in cases where they
> were previously included.
>
> I can't provide more details now, sorry. I will try to circle back to this
> later in the week if it is not obvious to you.
>
>
> On Wed, Jan 6, 2021 at 12:46 PM Greg Dove <gr...@gmail.com> wrote:
>
> > Oops, you already got there! Please ignore my previous comment :)
> >
> >
> > On Wed, Jan 6, 2021 at 12:39 PM <jo...@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
> >>
> >>
> >> The following commit(s) were added to refs/heads/develop by this push:
> >> new 084a639 AccessorEmitter: don't redeclare instance accessor as
> >> variable if it is an override
> >> 084a639 is described below
> >>
> >> commit 084a639759757024ea95db74b63ddaead33ee9d0
> >> Author: Josh Tynjala <jo...@apache.org>
> >> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
> >>
> >> AccessorEmitter: don't redeclare instance accessor as variable if it
> >> is an override
> >>
> >> Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
> >> ---
> >> .../internal/codegen/js/jx/AccessorEmitter.java | 71
> >> +++++++++++-----------
> >> .../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
> >> .../codegen/js/royale/TestRoyaleClass.java | 1 -
> >> .../resources/royale/projects/super/Base_result.js | 8 ---
> >> 4 files changed, 41 insertions(+), 51 deletions(-)
> >>
> >> 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 771a56c..2c193bc 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
> >> @@ -128,42 +128,45 @@ public class AccessorEmitter extends JSSubEmitter
> >> implements
> >> }
> >> else
> >> {
> >> - // start by writing out the instance accessors as
> >> regular variables
> >> - // because Closure Compiler doesn't properly
> analyze
> >> calls to
> >> - // defineProperties() alone.
> >> - // since there's no analysis, Closure assumes that
> >> getters/setters
> >> - // have no side effects, which results in important
> >> get/set calls
> >> - // being removed as dead code.
> >> - // defining the accessors as variables first
> >> convinces Closure to
> >> - // handle them more intelligently while not
> >> preventing them from
> >> - // being real accessors.
> >> - // Source:
> >> https://developers.google.com/closure/compiler/docs/limitations
> >> - writeNewline();
> >> - writeNewline();
> >> - writeNewline();
> >> - writeNewline("/**");
> >> - if (p.preventRename)
> >> - writeNewline(" * @nocollapse");
> >> - if (p.resolvedExport && !p.suppressExport)
> >> - writeNewline(" * @export");
> >> - if (p.type != null)
> >> - writeNewline(" * @type {" +
> >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> >> p.type.getPackageName()) + "}");
> >> - writeNewline(" */");
> >> - write(getEmitter().formatQualifiedName(qname));
> >> - write(ASEmitterTokens.MEMBER_ACCESS);
> >> - write(JSEmitterTokens.PROTOTYPE);
> >> - write(ASEmitterTokens.MEMBER_ACCESS);
> >> - if (p.uri != null)
> >> + IAccessorNode accessorNode = (getterNode != null) ?
> >> getterNode : setterNode;
> >> + if(!accessorNode.getDefinition().isOverride())
> >> {
> >> - IAccessorNode node = (getterNode != null) ?
> >> getterNode : setterNode;
> >> - INamespaceDecorationNode ns =
> >> ((FunctionNode)node).getActualNamespaceNode();
> >> - INamespaceDefinition nsDef =
> >> (INamespaceDefinition)ns.resolve(project);
> >> -
> >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
> >> names
> >> -
> >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
> >> + // start by writing out the instance accessors
> >> as regular variables
> >> + // because Closure Compiler doesn't properly
> >> analyze calls to
> >> + // defineProperties() alone.
> >> + // since there's no analysis, Closure assumes
> >> that getters/setters
> >> + // have no side effects, which results in
> >> important get/set calls
> >> + // being removed as dead code.
> >> + // defining the accessors as variables first
> >> convinces Closure to
> >> + // handle them more intelligently while not
> >> preventing them from
> >> + // being real accessors.
> >> + // Source:
> >> https://developers.google.com/closure/compiler/docs/limitations
> >> + writeNewline();
> >> + writeNewline();
> >> + writeNewline();
> >> + writeNewline("/**");
> >> + if (p.preventRename)
> >> + writeNewline(" * @nocollapse");
> >> + if (p.resolvedExport && !p.suppressExport)
> >> + writeNewline(" * @export");
> >> + if (p.type != null)
> >> + writeNewline(" * @type {" +
> >> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> >> p.type.getPackageName()) + "}");
> >> + writeNewline(" */");
> >> + write(getEmitter().formatQualifiedName(qname));
> >> + write(ASEmitterTokens.MEMBER_ACCESS);
> >> + write(JSEmitterTokens.PROTOTYPE);
> >> + write(ASEmitterTokens.MEMBER_ACCESS);
> >> + if (p.uri != null)
> >> + {
> >> + INamespaceDecorationNode ns =
> >> ((FunctionNode) accessorNode).getActualNamespaceNode();
> >> + INamespaceDefinition nsDef =
> >> (INamespaceDefinition)ns.resolve(project);
> >> +
> >> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
> >> names
> >> +
> >> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
> >> + }
> >> + else
> >> + write(baseName);
> >> + write(ASEmitterTokens.SEMICOLON);
> >> }
> >> - else
> >> - write(baseName);
> >> - write(ASEmitterTokens.SEMICOLON);
> >>
> >> if (getterNode != null)
> >> {
> >> diff --git
> >>
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> >>
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> >> index c8d4c3a..958902d 100644
> >> ---
> >>
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> >> +++
> >>
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> >> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
> >> TestGoogAccessorMembers
> >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> >> asBlockWalker.visitClass(node);
> >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> >> A);\n\n\n" +
> >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> >> */\nB.prototype.foo;\n\n\n" +
> >> - "B.prototype.get__foo = function() {\n
> >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> >> + "B.prototype.get__foo = function() {\n return
> >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> >> "Object.defineProperties(B.prototype, /** @lends
> >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> >> B.prototype.get__foo}}\n);");
> >> }
> >>
> >> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
> >> TestGoogAccessorMembers
> >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> >> asBlockWalker.visitClass(node);
> >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> >> A);\n\n\n" +
> >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> >> */\nB.prototype.foo;\n\n\n" +
> >> - "B.prototype.get__foo = function() {\n
> >> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> >> + "B.prototype.get__foo = function() {\n return
> >> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> >> "Object.defineProperties(B.prototype, /** @lends
> >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> >> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
> >> }
> >>
> >> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
> >> TestGoogAccessorMembers
> >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> >> asBlockWalker.visitClass(node);
> >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> >> A);\n\n\n" +
> >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> >> */\nB.prototype.foo;\n\n\n" +
> >> - "B.prototype.set__foo = function(value)
> >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> >> + "B.prototype.set__foo = function(value) {\n
> >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> >> "Object.defineProperties(B.prototype, /** @lends
> >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
> >> B.prototype.set__foo}}\n);");
> >> }
> >>
> >> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
> >> TestGoogAccessorMembers
> >> IClassNode.class, WRAP_LEVEL_PACKAGE);
> >> asBlockWalker.visitClass(node);
> >> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> >> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> >> A);\n\n\n" +
> >> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> >> */\nB.prototype.foo;\n\n\n" +
> >> - "B.prototype.set__foo = function(value)
> >> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> >> + "B.prototype.set__foo = function(value) {\n
> >> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> >> "Object.defineProperties(B.prototype, /** @lends
> >> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> >> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
> >> }
> >>
> >> diff --git
> >>
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> >>
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> >> index a30202b..595dbdd 100644
> >> ---
> >>
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> >> +++
> >>
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> >> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
> >> IClassNode node = getClassNode("public class B extends A
> {public
> >> function B() {}; override public function set foo(value:Object):void
> >> {super.foo = value;};} class A {public function set
> foo(value:Object):void
> >> {}}");
> >> asBlockWalker.visitClass(node);
> >> String expected = "/**\n * @constructor\n * @extends
> >> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
> >> org.apache.royale.B.base(this,
> >> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
> >> org.apache.royale.A);\n\n\n" +
> >> - "/**\n * @nocollapse\n * @export\n * @type {Object}\n
> >> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
> >> "org.apache.royale.B.prototype.set__foo =
> >> function(value) {\n
> org.apache.royale.B.superClass_.set__foo.apply(this, [
> >> value] );\n};\n\n\n" +
> >> "Object.defineProperties(org.apache.royale.B.prototype,
> >> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type
> {Object}\n
> >> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
> >> assertOut(expected);
> >> diff --git
> >> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> >> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> >> index 169067a..fcaa48c 100644
> >> ---
> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> >> +++
> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> >> @@ -35,14 +35,6 @@ Base = function() {
> >> goog.inherits(Base, Super);
> >>
> >>
> >> -/**
> >> - * @nocollapse
> >> - * @export
> >> - * @type {string}
> >> - */
> >> -Base.prototype.text;
> >> -
> >> -
> >> Base.prototype.get__text = function() {
> >> return "A" + Base.superClass_.get__text.apply(this);
> >> };
> >>
> >>
>
Re: [royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
Posted by Greg Dove <gr...@gmail.com>.
Hi Josh,
That seems to fix that specific issue. But fyi I am seeing some differences
in goog.require output in some cases between 0.9.8-SNAPSHOT and
0.9.9-SNAPSHOT compiler build that might be what breaks things at runtime
in the project we are working on. Are these (or other recent changes)
supposed to change the goog.require part of the output?
I see changes in the commented out 'Royale Dependency List' and 'Royale
Static Dependency List' (the latest compiler code seems to have what were
previously 'static' dependencies moved to )
And I see items that were in the goog.requires missing in cases where they
were previously included.
I can't provide more details now, sorry. I will try to circle back to this
later in the week if it is not obvious to you.
On Wed, Jan 6, 2021 at 12:46 PM Greg Dove <gr...@gmail.com> wrote:
> Oops, you already got there! Please ignore my previous comment :)
>
>
> On Wed, Jan 6, 2021 at 12:39 PM <jo...@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
>>
>>
>> The following commit(s) were added to refs/heads/develop by this push:
>> new 084a639 AccessorEmitter: don't redeclare instance accessor as
>> variable if it is an override
>> 084a639 is described below
>>
>> commit 084a639759757024ea95db74b63ddaead33ee9d0
>> Author: Josh Tynjala <jo...@apache.org>
>> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
>>
>> AccessorEmitter: don't redeclare instance accessor as variable if it
>> is an override
>>
>> Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
>> ---
>> .../internal/codegen/js/jx/AccessorEmitter.java | 71
>> +++++++++++-----------
>> .../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
>> .../codegen/js/royale/TestRoyaleClass.java | 1 -
>> .../resources/royale/projects/super/Base_result.js | 8 ---
>> 4 files changed, 41 insertions(+), 51 deletions(-)
>>
>> 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 771a56c..2c193bc 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
>> @@ -128,42 +128,45 @@ public class AccessorEmitter extends JSSubEmitter
>> implements
>> }
>> else
>> {
>> - // start by writing out the instance accessors as
>> regular variables
>> - // because Closure Compiler doesn't properly analyze
>> calls to
>> - // defineProperties() alone.
>> - // since there's no analysis, Closure assumes that
>> getters/setters
>> - // have no side effects, which results in important
>> get/set calls
>> - // being removed as dead code.
>> - // defining the accessors as variables first
>> convinces Closure to
>> - // handle them more intelligently while not
>> preventing them from
>> - // being real accessors.
>> - // Source:
>> https://developers.google.com/closure/compiler/docs/limitations
>> - writeNewline();
>> - writeNewline();
>> - writeNewline();
>> - writeNewline("/**");
>> - if (p.preventRename)
>> - writeNewline(" * @nocollapse");
>> - if (p.resolvedExport && !p.suppressExport)
>> - writeNewline(" * @export");
>> - if (p.type != null)
>> - writeNewline(" * @type {" +
>> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
>> p.type.getPackageName()) + "}");
>> - writeNewline(" */");
>> - write(getEmitter().formatQualifiedName(qname));
>> - write(ASEmitterTokens.MEMBER_ACCESS);
>> - write(JSEmitterTokens.PROTOTYPE);
>> - write(ASEmitterTokens.MEMBER_ACCESS);
>> - if (p.uri != null)
>> + IAccessorNode accessorNode = (getterNode != null) ?
>> getterNode : setterNode;
>> + if(!accessorNode.getDefinition().isOverride())
>> {
>> - IAccessorNode node = (getterNode != null) ?
>> getterNode : setterNode;
>> - INamespaceDecorationNode ns =
>> ((FunctionNode)node).getActualNamespaceNode();
>> - INamespaceDefinition nsDef =
>> (INamespaceDefinition)ns.resolve(project);
>> -
>> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
>> names
>> -
>> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
>> + // start by writing out the instance accessors
>> as regular variables
>> + // because Closure Compiler doesn't properly
>> analyze calls to
>> + // defineProperties() alone.
>> + // since there's no analysis, Closure assumes
>> that getters/setters
>> + // have no side effects, which results in
>> important get/set calls
>> + // being removed as dead code.
>> + // defining the accessors as variables first
>> convinces Closure to
>> + // handle them more intelligently while not
>> preventing them from
>> + // being real accessors.
>> + // Source:
>> https://developers.google.com/closure/compiler/docs/limitations
>> + writeNewline();
>> + writeNewline();
>> + writeNewline();
>> + writeNewline("/**");
>> + if (p.preventRename)
>> + writeNewline(" * @nocollapse");
>> + if (p.resolvedExport && !p.suppressExport)
>> + writeNewline(" * @export");
>> + if (p.type != null)
>> + writeNewline(" * @type {" +
>> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
>> p.type.getPackageName()) + "}");
>> + writeNewline(" */");
>> + write(getEmitter().formatQualifiedName(qname));
>> + write(ASEmitterTokens.MEMBER_ACCESS);
>> + write(JSEmitterTokens.PROTOTYPE);
>> + write(ASEmitterTokens.MEMBER_ACCESS);
>> + if (p.uri != null)
>> + {
>> + INamespaceDecorationNode ns =
>> ((FunctionNode) accessorNode).getActualNamespaceNode();
>> + INamespaceDefinition nsDef =
>> (INamespaceDefinition)ns.resolve(project);
>> +
>> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
>> names
>> +
>> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
>> + }
>> + else
>> + write(baseName);
>> + write(ASEmitterTokens.SEMICOLON);
>> }
>> - else
>> - write(baseName);
>> - write(ASEmitterTokens.SEMICOLON);
>>
>> if (getterNode != null)
>> {
>> diff --git
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> index c8d4c3a..958902d 100644
>> ---
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> +++
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
>> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
>> TestGoogAccessorMembers
>> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> asBlockWalker.visitClass(node);
>> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> A);\n\n\n" +
>> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> */\nB.prototype.foo;\n\n\n" +
>> - "B.prototype.get__foo = function() {\n
>> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> + "B.prototype.get__foo = function() {\n return
>> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> "Object.defineProperties(B.prototype, /** @lends
>> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> B.prototype.get__foo}}\n);");
>> }
>>
>> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
>> TestGoogAccessorMembers
>> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> asBlockWalker.visitClass(node);
>> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> A);\n\n\n" +
>> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> */\nB.prototype.foo;\n\n\n" +
>> - "B.prototype.get__foo = function() {\n
>> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> + "B.prototype.get__foo = function() {\n return
>> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>> "Object.defineProperties(B.prototype, /** @lends
>> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
>> }
>>
>> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
>> TestGoogAccessorMembers
>> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> asBlockWalker.visitClass(node);
>> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> A);\n\n\n" +
>> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> */\nB.prototype.foo;\n\n\n" +
>> - "B.prototype.set__foo = function(value)
>> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> + "B.prototype.set__foo = function(value) {\n
>> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> "Object.defineProperties(B.prototype, /** @lends
>> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
>> B.prototype.set__foo}}\n);");
>> }
>>
>> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
>> TestGoogAccessorMembers
>> IClassNode.class, WRAP_LEVEL_PACKAGE);
>> asBlockWalker.visitClass(node);
>> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
>> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
>> A);\n\n\n" +
>> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
>> */\nB.prototype.foo;\n\n\n" +
>> - "B.prototype.set__foo = function(value)
>> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> + "B.prototype.set__foo = function(value) {\n
>> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>> "Object.defineProperties(B.prototype, /** @lends
>> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
>> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
>> }
>>
>> diff --git
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> index a30202b..595dbdd 100644
>> ---
>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> +++
>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
>> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
>> IClassNode node = getClassNode("public class B extends A {public
>> function B() {}; override public function set foo(value:Object):void
>> {super.foo = value;};} class A {public function set foo(value:Object):void
>> {}}");
>> asBlockWalker.visitClass(node);
>> String expected = "/**\n * @constructor\n * @extends
>> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
>> org.apache.royale.B.base(this,
>> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
>> org.apache.royale.A);\n\n\n" +
>> - "/**\n * @nocollapse\n * @export\n * @type {Object}\n
>> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
>> "org.apache.royale.B.prototype.set__foo =
>> function(value) {\n org.apache.royale.B.superClass_.set__foo.apply(this, [
>> value] );\n};\n\n\n" +
>> "Object.defineProperties(org.apache.royale.B.prototype,
>> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type {Object}\n
>> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
>> assertOut(expected);
>> diff --git
>> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> index 169067a..fcaa48c 100644
>> --- a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> +++ b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
>> @@ -35,14 +35,6 @@ Base = function() {
>> goog.inherits(Base, Super);
>>
>>
>> -/**
>> - * @nocollapse
>> - * @export
>> - * @type {string}
>> - */
>> -Base.prototype.text;
>> -
>> -
>> Base.prototype.get__text = function() {
>> return "A" + Base.superClass_.get__text.apply(this);
>> };
>>
>>
Re: [royale-compiler] branch develop updated: AccessorEmitter: don't
redeclare instance accessor as variable if it is an override
Posted by Greg Dove <gr...@gmail.com>.
Oops, you already got there! Please ignore my previous comment :)
On Wed, Jan 6, 2021 at 12:39 PM <jo...@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
>
>
> The following commit(s) were added to refs/heads/develop by this push:
> new 084a639 AccessorEmitter: don't redeclare instance accessor as
> variable if it is an override
> 084a639 is described below
>
> commit 084a639759757024ea95db74b63ddaead33ee9d0
> Author: Josh Tynjala <jo...@apache.org>
> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
>
> AccessorEmitter: don't redeclare instance accessor as variable if it
> is an override
>
> Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
> ---
> .../internal/codegen/js/jx/AccessorEmitter.java | 71
> +++++++++++-----------
> .../js/royale/TestRoyaleAccessorMembers.java | 12 ++--
> .../codegen/js/royale/TestRoyaleClass.java | 1 -
> .../resources/royale/projects/super/Base_result.js | 8 ---
> 4 files changed, 41 insertions(+), 51 deletions(-)
>
> 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 771a56c..2c193bc 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
> @@ -128,42 +128,45 @@ public class AccessorEmitter extends JSSubEmitter
> implements
> }
> else
> {
> - // start by writing out the instance accessors as
> regular variables
> - // because Closure Compiler doesn't properly analyze
> calls to
> - // defineProperties() alone.
> - // since there's no analysis, Closure assumes that
> getters/setters
> - // have no side effects, which results in important
> get/set calls
> - // being removed as dead code.
> - // defining the accessors as variables first
> convinces Closure to
> - // handle them more intelligently while not
> preventing them from
> - // being real accessors.
> - // Source:
> https://developers.google.com/closure/compiler/docs/limitations
> - writeNewline();
> - writeNewline();
> - writeNewline();
> - writeNewline("/**");
> - if (p.preventRename)
> - writeNewline(" * @nocollapse");
> - if (p.resolvedExport && !p.suppressExport)
> - writeNewline(" * @export");
> - if (p.type != null)
> - writeNewline(" * @type {" +
> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> p.type.getPackageName()) + "}");
> - writeNewline(" */");
> - write(getEmitter().formatQualifiedName(qname));
> - write(ASEmitterTokens.MEMBER_ACCESS);
> - write(JSEmitterTokens.PROTOTYPE);
> - write(ASEmitterTokens.MEMBER_ACCESS);
> - if (p.uri != null)
> + IAccessorNode accessorNode = (getterNode != null) ?
> getterNode : setterNode;
> + if(!accessorNode.getDefinition().isOverride())
> {
> - IAccessorNode node = (getterNode != null) ?
> getterNode : setterNode;
> - INamespaceDecorationNode ns =
> ((FunctionNode)node).getActualNamespaceNode();
> - INamespaceDefinition nsDef =
> (INamespaceDefinition)ns.resolve(project);
> -
> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
> names
> -
> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
> + // start by writing out the instance accessors as
> regular variables
> + // because Closure Compiler doesn't properly
> analyze calls to
> + // defineProperties() alone.
> + // since there's no analysis, Closure assumes
> that getters/setters
> + // have no side effects, which results in
> important get/set calls
> + // being removed as dead code.
> + // defining the accessors as variables first
> convinces Closure to
> + // handle them more intelligently while not
> preventing them from
> + // being real accessors.
> + // Source:
> https://developers.google.com/closure/compiler/docs/limitations
> + writeNewline();
> + writeNewline();
> + writeNewline();
> + writeNewline("/**");
> + if (p.preventRename)
> + writeNewline(" * @nocollapse");
> + if (p.resolvedExport && !p.suppressExport)
> + writeNewline(" * @export");
> + if (p.type != null)
> + writeNewline(" * @type {" +
> JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(),
> p.type.getPackageName()) + "}");
> + writeNewline(" */");
> + write(getEmitter().formatQualifiedName(qname));
> + write(ASEmitterTokens.MEMBER_ACCESS);
> + write(JSEmitterTokens.PROTOTYPE);
> + write(ASEmitterTokens.MEMBER_ACCESS);
> + if (p.uri != null)
> + {
> + INamespaceDecorationNode ns = ((FunctionNode)
> accessorNode).getActualNamespaceNode();
> + INamespaceDefinition nsDef =
> (INamespaceDefinition)ns.resolve(project);
> +
> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
> names
> +
> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
> + }
> + else
> + write(baseName);
> + write(ASEmitterTokens.SEMICOLON);
> }
> - else
> - write(baseName);
> - write(ASEmitterTokens.SEMICOLON);
>
> if (getterNode != null)
> {
> diff --git
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> index c8d4c3a..958902d 100644
> ---
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> +++
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
> IClassNode.class, WRAP_LEVEL_PACKAGE);
> asBlockWalker.visitClass(node);
> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> - "B.prototype.get__foo = function() {\n
> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> + "B.prototype.get__foo = function() {\n return
> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> B.prototype.get__foo}}\n);");
> }
>
> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
> IClassNode.class, WRAP_LEVEL_PACKAGE);
> asBlockWalker.visitClass(node);
> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> - "B.prototype.get__foo = function() {\n
> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> + "B.prototype.get__foo = function() {\n return
> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
> }
>
> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
> IClassNode.class, WRAP_LEVEL_PACKAGE);
> asBlockWalker.visitClass(node);
> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> - "B.prototype.set__foo = function(value)
> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> + "B.prototype.set__foo = function(value) {\n
> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
> B.prototype.set__foo}}\n);");
> }
>
> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
> IClassNode.class, WRAP_LEVEL_PACKAGE);
> asBlockWalker.visitClass(node);
> assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> - "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> - "B.prototype.set__foo = function(value)
> {\n B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> + "B.prototype.set__foo = function(value) {\n
> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
> }
>
> diff --git
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> index a30202b..595dbdd 100644
> ---
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> +++
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
> IClassNode node = getClassNode("public class B extends A {public
> function B() {}; override public function set foo(value:Object):void
> {super.foo = value;};} class A {public function set foo(value:Object):void
> {}}");
> asBlockWalker.visitClass(node);
> String expected = "/**\n * @constructor\n * @extends
> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
> org.apache.royale.B.base(this,
> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
> org.apache.royale.A);\n\n\n" +
> - "/**\n * @nocollapse\n * @export\n * @type {Object}\n
> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
> "org.apache.royale.B.prototype.set__foo = function(value)
> {\n org.apache.royale.B.superClass_.set__foo.apply(this, [ value]
> );\n};\n\n\n" +
> "Object.defineProperties(org.apache.royale.B.prototype,
> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type {Object}\n
> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
> assertOut(expected);
> diff --git
> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> index 169067a..fcaa48c 100644
> --- a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> +++ b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> @@ -35,14 +35,6 @@ Base = function() {
> goog.inherits(Base, Super);
>
>
> -/**
> - * @nocollapse
> - * @export
> - * @type {string}
> - */
> -Base.prototype.text;
> -
> -
> Base.prototype.get__text = function() {
> return "A" + Base.superClass_.get__text.apply(this);
> };
>
>