Sorry I only followed this on the email pings, and I did see the commit message Josh, but I was wondering how and why there was a need to reflect into the 'private' class in the first place. Obviously RoyaleUnit does that somewhere, thanks for explaining.
Perhaps it can still be possible to add an optimization because I imagine this is an unusual case for reflection although the optimization could be a little complicated I guess. For example, if there is never anything in the public class that publicly exposes a private class or instance of a private class, then I don't expect that local private class would need reflection data. Seems low priority, but I will keep it in mind. btw congrats on your progress with RoyaleUnit. So pleased to see that. When I originally worked on reflection stuff a few years back it was with the goal of getting some sort of unit testing approach in place. I set up something very basic that I find useful for quick iterative comparisons between JS and SWF, based on an experience I had a number of years ago working with multiple (8+) Haxe targets and running those unit tests for all of them side by side with a browser view of all results. It was a very accessible way to contribute to, in many cases, multiple targets. I think they may have changed how that works now, not sure, I believe they also have more CI focus now. Anyhow, I should have a lot of unit tests that will hopefully be mostly compatible and therefore easily ported to your RoyaleUnit - they were originally set up to be compatible with FlexUnit and I was able to copy/paste into the FlexUnit setup, but some of the Test classes I am using do have allowances for known variance between for JS and SWF. I will try RoyaleUnit soon and see if can port some of the tests from manualtests/UnitTests On Thu, May 23, 2019 at 10:30 AM Josh Tynjala <[email protected]> wrote: > Hi Greg, > > Indeed, supporting describeType() is exactly why I made this change. > > From my commit message: > > > Fixes issue where describeType() could not be used on file-internal > > classes because the reflection info was missing > > I discovered this issue when trying to get the framework's RoyaleUnit > tests to run in JS with the <royaleunit> Ant task. RoyaleUnit uses > reflection to detect metadata on classes. > > In the tests that RoyaleUnit runs on itself, I created some mock test > classes that are file-internal: > > > https://github.com/apache/royale-asjs/blob/develop/frameworks/projects/RoyaleUnit/src/test/royale/tests/BeforeAndAfterTests.as > > Without this reflection data, the app crashes in JS because > ROYALE_REFLECTION_DATA is missing. This code has been working fine in SWF > for a while now, so I had no reason to believe it would break when I > finally got around to building the same app for JS. > > - Josh > > On 2019/05/22 22:10:54, Greg Dove <[email protected]> wrote: > > Actually there is one exception I thought of, and verified .... > > > > describeType works in avm if the private class is exposed via the public > > class > > new TestClass().getPrivateClass() > > > > But getDefinitionByName > > and ApplicationDomain.currentDomain.getQualifiedDefinitionNames() do not > > seem to know about it. > > > > Anyway, keen to hear more. If it's needed so be it (in which case my > > original attempt to optimize was invalid). > > > > > > > > On Thu, May 23, 2019 at 9:58 AM Greg Dove <[email protected]> wrote: > > > > > > > > I just did a quick test using Adobe animate: > > > trace(ApplicationDomain.currentDomain.getQualifiedDefinitionNames()) > > > > > > The private internal classes are not available generally. > > > > > > If I add a public member to the public class in the file that has a > type > > > corresponding to a file private class, the string name of the private > class > > > is represented via describeType. But it throws an error if I try to use > > > that subsequently via getDefinitionByName. Maybe there is something > extra I > > > need to do to somehow make it work, but so far, on the face of it, it > seems > > > that reflection data should not be needed for 'file private' or 'file > > > internal' classes. > > > > > > > > > > > > On Thu, May 23, 2019 at 9:35 AM Greg Dove <[email protected]> wrote: > > > > > >> > > >> Hi Josh, > > >> > > >> Is that change necessary? As part of the work I did previously on > > >> reflection I actually removed reflection data from private internal > classes > > >> as an optimization, because I did not expect there would not be a > need to > > >> reflect into file 'private' classes. > > >> I don't think this is possible in avm? (Might be wrong, but I have > > >> certainly never knowingly done it) > > >> > > >> > > >> > > >> On Thu, May 23, 2019 at 9:04 AM <[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 b533c38 PackageFooterEmitter: added missing > > >>> ROYALE_REFLECTION_INFO for file-internal classes > > >>> b533c38 is described below > > >>> > > >>> commit b533c389d9335ce1a17b2fd1241dc1e5e6016bda > > >>> Author: Josh Tynjala <[email protected]> > > >>> AuthorDate: Wed May 22 14:04:27 2019 -0700 > > >>> > > >>> PackageFooterEmitter: added missing ROYALE_REFLECTION_INFO for > > >>> file-internal classes > > >>> > > >>> Fixes issue where describeType() could not be used on > file-internal > > >>> classes because the reflection info was missing > > >>> --- > > >>> .../codegen/js/jx/PackageFooterEmitter.java | 19 ++- > > >>> .../codegen/js/royale/TestRoyalePackage.java | 156 > > >>> ++++++++++++++++++++- > > >>> .../royale/projects/internal/MainClass_result.js | 29 ++++ > > >>> 3 files changed, 188 insertions(+), 16 deletions(-) > > >>> > > >>> diff --git > > >>> > a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageFooterEmitter.java > > >>> > b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageFooterEmitter.java > > >>> index fec56eb..7c151ac 100644 > > >>> --- > > >>> > a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageFooterEmitter.java > > >>> +++ > > >>> > b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageFooterEmitter.java > > >>> @@ -80,7 +80,6 @@ public class PackageFooterEmitter extends > JSSubEmitter > > >>> implements > > >>> { > > >>> boolean isInterface = tnode instanceof > > >>> IInterfaceNode; > > >>> boolean isDynamic = tnode instanceof > IClassNode > > >>> && tnode.hasModifier(ASModifier.DYNAMIC); > > >>> - boolean isInternalClass = !isInterface && tnode > > >>> instanceof IClassNode && > > >>> getEmitter().getModel().isInternalClass(tnode.getQualifiedName()); > > >>> /* > > >>> * Metadata > > >>> * > > >>> @@ -199,17 +198,15 @@ public class PackageFooterEmitter extends > > >>> JSSubEmitter implements > > >>> > > >>> String typeName = > > >>> getEmitter().formatQualifiedName(tnode.getQualifiedName()); > > >>> > > >>> - if (!isInternalClass) { > > >>> - emitReflectionData( > > >>> - typeName, > > >>> - reflectionKind, > > >>> - varData, > > >>> - accessorData, > > >>> - methodData, > > >>> - metadata); > > >>> - } > > >>> + emitReflectionData( > > >>> + typeName, > > >>> + reflectionKind, > > >>> + varData, > > >>> + accessorData, > > >>> + methodData, > > >>> + metadata); > > >>> > > >>> - if (!isInterface && !isInternalClass) { > > >>> + if (!isInterface) { > > >>> > > >>> emitReflectionRegisterInitialStaticFields(typeName, (ClassDefinition) > > >>> tnode.getDefinition()); > > >>> } > > >>> > > >>> diff --git > > >>> > a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java > > >>> > b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java > > >>> index 488954a..2d9166c 100644 > > >>> --- > > >>> > a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java > > >>> +++ > > >>> > b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java > > >>> @@ -531,7 +531,32 @@ public class TestRoyalePackage extends > > >>> TestGoogPackage > > >>> " *\n" + > > >>> " * @type {Object.<string, > > >>> Array.<Object>>}\n" + > > >>> " */\n" + > > >>> - > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };" > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "/**\n" + > > >>> + " * Reflection\n" + > > >>> + " *\n" + > > >>> + " * @return {Object.<string, > > >>> Function>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO = > function > > >>> () {\n" + > > >>> + " return {\n" + > > >>> + " variables: function () {return > > >>> {};},\n" + > > >>> + " accessors: function () {return > > >>> {};},\n" + > > >>> + " methods: function () {\n" + > > >>> + " return {\n" + > > >>> + " 'InternalClass': { type: '', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'}\n" + > > >>> + " };\n" + > > >>> + " }\n" + > > >>> + " };\n" + > > >>> + "};\n" + > > >>> + "/**\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {number}\n" + > > >>> + " */\n" + > > >>> + > > >>> > "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO.compileFlags > > >>> = 15;\n" > > >>> ); > > >>> } > > >>> > > >>> @@ -815,7 +840,45 @@ public class TestRoyalePackage extends > > >>> TestGoogPackage > > >>> " *\n" + > > >>> " * @type {Object.<string, > > >>> Array.<Object>>}\n" + > > >>> " */\n" + > > >>> - > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };" > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "/**\n" + > > >>> + " * Reflection\n" + > > >>> + " *\n" + > > >>> + " * @return {Object.<string, > > >>> Function>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO = > function > > >>> () {\n" + > > >>> + " return {\n" + > > >>> + " variables: function () {\n" + > > >>> + " return {\n" + > > >>> + " '|someString': { type: > > >>> 'String', get_set: function (/** * */ v) {return v !== undefined ? > > >>> foo.bar.baz.A.InternalClass.someString = v : > > >>> foo.bar.baz.A.InternalClass.someString;}}\n" + > > >>> + " };\n" + > > >>> + " },\n" + > > >>> + " accessors: function () {return > > >>> {};},\n" + > > >>> + " methods: function () {\n" + > > >>> + " return {\n" + > > >>> + " 'InternalClass': { type: '', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'},\n" + > > >>> + " '|someStaticFunction': { > type: > > >>> 'String', declaredBy: 'foo.bar.baz.A.InternalClass'},\n" + > > >>> + " 'someMethod': { type: > 'String', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'}\n" + > > >>> + " };\n" + > > >>> + " }\n" + > > >>> + " };\n" + > > >>> + "};\n" + > > >>> + "/**\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {number}\n" + > > >>> + " */\n" + > > >>> + > > >>> > "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO.compileFlags > > >>> = 15;\n" + > > >>> + "/**\n" + > > >>> + " * Provide reflection support for > > >>> distinguishing dynamic fields on class object (static)\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {Array<string>}\n" + > > >>> + " */\n" + > > >>> + > > >>> > "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO.statics = > > >>> Object.keys(foo.bar.baz.A.InternalClass);\n" > > >>> ); > > >>> } > > >>> > > >>> @@ -966,7 +1029,36 @@ public class TestRoyalePackage extends > > >>> TestGoogPackage > > >>> " *\n" + > > >>> " * @type {Object.<string, > > >>> Array.<Object>>}\n" + > > >>> " */\n" + > > >>> - > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };" > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }] };\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "/**\n" + > > >>> + " * Reflection\n" + > > >>> + " *\n" + > > >>> + " * @return {Object.<string, > > >>> Function>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO = > function > > >>> () {\n" + > > >>> + " return {\n" + > > >>> + " variables: function () {return > > >>> {};},\n" + > > >>> + " accessors: function () {\n" + > > >>> + " return {\n" + > > >>> + " 'someString': { type: > 'String', > > >>> access: 'readwrite', declaredBy: 'foo.bar.baz.A.InternalClass'}\n" + > > >>> + " };\n" + > > >>> + " },\n" + > > >>> + " methods: function () {\n" + > > >>> + " return {\n" + > > >>> + " 'InternalClass': { type: '', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'}\n" + > > >>> + " };\n" + > > >>> + " }\n" + > > >>> + " };\n" + > > >>> + "};\n" + > > >>> + "/**\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {number}\n" + > > >>> + " */\n" + > > >>> + > > >>> > "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO.compileFlags > > >>> = 15;\n" > > >>> ); > > >>> } > > >>> > > >>> @@ -1126,7 +1218,33 @@ public class TestRoyalePackage extends > > >>> TestGoogPackage > > >>> " *\n" + > > >>> " * @type {Object.<string, > > >>> Array.<Object>>}\n" + > > >>> " */\n" + > > >>> - > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }], interfaces: [foo.bar.baz.A.ITestInterface] };" > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_CLASS_INFO = { names: > [{ > > >>> name: 'InternalClass', qName: 'foo.bar.baz.A.InternalClass', kind: > 'class' > > >>> }], interfaces: [foo.bar.baz.A.ITestInterface] };\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "/**\n" + > > >>> + " * Reflection\n" + > > >>> + " *\n" + > > >>> + " * @return {Object.<string, > > >>> Function>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO = > function > > >>> () {\n" + > > >>> + " return {\n" + > > >>> + " variables: function () {return > > >>> {};},\n" + > > >>> + " accessors: function () {return > > >>> {};},\n" + > > >>> + " methods: function () {\n" + > > >>> + " return {\n" + > > >>> + " 'InternalClass': { type: '', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'},\n" + > > >>> + " 'test': { type: 'void', > > >>> declaredBy: 'foo.bar.baz.A.InternalClass'}\n" + > > >>> + " };\n" + > > >>> + " }\n" + > > >>> + " };\n" + > > >>> + "};\n" + > > >>> + "/**\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {number}\n" + > > >>> + " */\n" + > > >>> + > > >>> > "foo.bar.baz.A.InternalClass.prototype.ROYALE_REFLECTION_INFO.compileFlags > > >>> = 15;\n" > > >>> ); > > >>> } > > >>> > > >>> @@ -1189,7 +1307,35 @@ public class TestRoyalePackage extends > > >>> TestGoogPackage > > >>> " *\n" + > > >>> " * @type {Object.<string, > > >>> Array.<Object>>}\n" + > > >>> " */\n" + > > >>> - > > >>> "foo.bar.A.Internal.prototype.ROYALE_CLASS_INFO = { names: [{ name: > > >>> 'Internal', qName: 'foo.bar.A.Internal', kind: 'class' }] };" > > >>> + > > >>> "foo.bar.A.Internal.prototype.ROYALE_CLASS_INFO = { names: [{ name: > > >>> 'Internal', qName: 'foo.bar.A.Internal', kind: 'class' }] };\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "\n" + > > >>> + "/**\n" + > > >>> + " * Reflection\n" + > > >>> + " *\n" + > > >>> + " * @return {Object.<string, > > >>> Function>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.A.Internal.prototype.ROYALE_REFLECTION_INFO = function () > {\n" + > > >>> + " return {\n" + > > >>> + " variables: function () {return > > >>> {};},\n" + > > >>> + " accessors: function () {return > > >>> {};},\n" + > > >>> + " methods: function () {return > > >>> {};}\n" + > > >>> + " };\n" + > > >>> + "};\n" + > > >>> + "/**\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {number}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.A.Internal.prototype.ROYALE_REFLECTION_INFO.compileFlags = > 15;\n" > > >>> + > > >>> + "/**\n" + > > >>> + " * Provide reflection support for > > >>> distinguishing dynamic fields on class object (static)\n" + > > >>> + " * @export\n" + > > >>> + " * @const\n" + > > >>> + " * @type {Array<string>}\n" + > > >>> + " */\n" + > > >>> + > > >>> "foo.bar.A.Internal.prototype.ROYALE_REFLECTION_INFO.statics = > > >>> Object.keys(foo.bar.A.Internal);\n" > > >>> ); > > >>> } > > >>> > > >>> diff --git > > >>> > a/compiler-jx/src/test/resources/royale/projects/internal/MainClass_result.js > > >>> > b/compiler-jx/src/test/resources/royale/projects/internal/MainClass_result.js > > >>> index f2bff9f..d403d5f 100644 > > >>> --- > > >>> > a/compiler-jx/src/test/resources/royale/projects/internal/MainClass_result.js > > >>> +++ > > >>> > b/compiler-jx/src/test/resources/royale/projects/internal/MainClass_result.js > > >>> @@ -100,3 +100,32 @@ MainClass.InternalClass.prototype.foo = null; > > >>> * @type {Object.<string, Array.<Object>>} > > >>> */ > > >>> MainClass.InternalClass.prototype.ROYALE_CLASS_INFO = { names: [{ > name: > > >>> 'InternalClass', qName: 'MainClass.InternalClass', kind: 'class' }] > }; > > >>> + > > >>> + > > >>> + > > >>> +/** > > >>> + * Reflection > > >>> + * > > >>> + * @return {Object.<string, Function>} > > >>> + */ > > >>> +MainClass.InternalClass.prototype.ROYALE_REFLECTION_INFO = function > () { > > >>> + return { > > >>> + variables: function () { > > >>> + return { > > >>> + 'foo': { type: 'OtherClass', get_set: function (/** > > >>> MainClass.InternalClass */ inst, /** * */ v) {return v !== undefined > ? > > >>> inst.foo = v : inst.foo;}} > > >>> + }; > > >>> + }, > > >>> + accessors: function () {return {};}, > > >>> + methods: function () { > > >>> + return { > > >>> + 'InternalClass': { type: '', declaredBy: > > >>> 'MainClass.InternalClass'} > > >>> + }; > > >>> + } > > >>> + }; > > >>> +}; > > >>> +/** > > >>> + * @export > > >>> + * @const > > >>> + * @type {number} > > >>> + */ > > >>> > +MainClass.InternalClass.prototype.ROYALE_REFLECTION_INFO.compileFlags = > > >>> 9; > > >>> \ No newline at end of file > > >>> > > >>> > > >
