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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> wrote: >> > >> > > Oops, you already got there! Please ignore my previous comment :) >> > > >> > > >> > > On Wed, Jan 6, 2021 at 12:39 PM <[email protected]> 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 <[email protected]> >> > >> 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); >> > >> }; >> > >> >> > >> >> > >> >
