Actually, I hadn't considered the issue you just brought up about the inheritance chain changing in a separately compiled project. I would say that is a valid concern and probably means that "depth" approach is not the right one. I was mostly concerned about compiler performance and whether the compiler has efficient access to the depth if the base classes are in separately compiled project. The compiler is already not as fast as we hoped and we need to be careful about adding loops that are expensive.
FWIW, a while ago there was another discussion about output size and the number of long strings related to package names. One option is to introduce an "alias" table in the compiler. You can give it package names and a short name to substitute for it. So if we say that org.apache.royale.core will be replaced by "oarc" then the decorated name for foo would be _foo_oarcBaseClass and _foo_oarcMyClass I think that's better than $P1 and $P2 because it does help to know which class's private variables you are looking at without knowing the chain of baseclasses. And elsewhere the download size for Royale apps will shrink. Thoughts? -Alex On 11/12/18, 10:41 AM, "Greg Dove" <[email protected]> wrote: btw I did not get to this last week like I hoped. But I did look at this a bit yesterday and I have a local branch that does this for the original example at the beginning of this thread (which I used as my one 'real' example for local testing): org.apache.royale.core.BaseClass = function() { org.apache.royale.utils.Language.trace(this._foo_$P1); }; org.apache.royale.core.BaseClass.prototype._foo_$P1 = "foo,"; and org.apache.royale.core.MyClass = function() { org.apache.royale.core.MyClass.base(this, 'constructor'); org.apache.royale.utils.Language.trace(this._foo_$P2); }; org.apache.royale.core.MyClass.prototype._foo_$P2 = "bar!"; (ommitted some output for clarity) That correctly separates both private foo members at the two levels, and gives the expected output. But I need to understand what the concerns are that you mentioned about swcs, Alex. You've using globally unique names for the solution which is always going to work, I'm trying to find a way to limit the solution to the nature of the conflicts which exist only in the inheritance chain - 'depth' in the chain (the delta from 'Object' as a base class) seemed like a good modifier for a 'private namespace', because in theory it would never conflict with ancestors. I guess some stale swcs where swcs get updated independently and where class hierarchies change in one that another depends on could end up with conflicts reappearing. The above approach could be an issue if one older swc uses a base class from another swc that became modified and gained a longer inheritance chain than it did when the first swc was made. Is that the type of thing you meant? On Tue, Nov 13, 2018 at 6:38 AM Alex Harui <[email protected]> wrote: > Maybe. Certainly at the time we compile it, we don't know if there will > be a subclass. > > I hopefully put in a check so that you can't "hide" a public or protected > API with a private one, but I think I left it so that a subclass can create > a public API with the same name as a private API. I don't know if that was > actually allowed in SWF, maybe you can test it out. But if that is > allowed, then the "top-level" must have private APIs with unique names. > > Of course, I could be wrong... > -Alex > > On 11/12/18, 7:27 AM, "Harbs" <[email protected]> wrote: > > I’m seeing private vars that are not subclassed also with qualified > names. It seems to me that top-level private vars have no reason to be > qualified.My $0.02, > Harbs > > > On Nov 6, 2018, at 11:03 PM, Alex Harui <[email protected]> > wrote: > > > > It would probably work, but why chase the inheritance chain at all? > > > > Go ahead and add that if you really want to, but I would rather we > not use colliding private names in the framework, and only one person spoke > up to say they use colliding private names in their code. > > > > Historically, folks really hated any private APIs in the Flex > framework. Really, we should only use them as backing variables of > getter/setters and for APIs we aren't sure we want to support going > forward. And even the backing variables may get exposed as protected (some > already are). Over time, private API implementations should stabilize and > then should be supported by making them protected. > > > > So, do what you want to do, but I'm not sure how many customers > there are. And if you do try to do this, make sure you can walk the entire > hierarchy across SWCs. I think it should work, but that could be a place > where you get stuck. > > > > Thanks, > > -Alex > > > > On 11/6/18, 11:30 AM, "Greg Dove" <[email protected]> wrote: > > > > 'I'm not sure it is worth figuring out how far up the inheritance > chain the > > other private variable is and assigning numbers ' > > > > I was just meaning at simple class level - so not doing anything > more than > > that. The 'depth number' is the same for all private fields > > Y extends Object > > X extends Y > > > > in Y all privates are _$P1 suffix > > in X all privates are _$P2 suffix > > > > nothing too sophisticated > > > > would that work? > > > > > > On Wed, Nov 7, 2018 at 7:39 AM Alex Harui > <[email protected]> wrote: > > > >> Hi Greg, > >> > >> #1 is a good idea and will help, but the value for that variable is > still > >> pushed far to the right in the debugger and pushed other code far > to the > >> right in the source window. > >> > >> #2 and similar ideas will help. I'm not sure it is worth figuring > out how > >> far up the inheritance chain the other private variable is and > assigning > >> numbers. I considered using some sort of hash of the fully > qualified > >> classname so each prefix/suffix would be, say 8 characters long. > >> > >> That said, though, in the framework, I think we should not have > private > >> name collisions. I found some duplicate code that would be caught > if we > >> turned on errors for these collisions, and would encourage us to use > >> overriding which enables other folks to do overrides. > >> > >> So, if you want to muck around in this code, feel free to switch > the order > >> in #1, but I think the next piece of work is to turn on errors for > >> collisions in the royale-asjs build. > >> > >> My 2 cents, > >> -Alex > >> > >> On 11/6/18, 10:07 AM, "Greg Dove" <[email protected]> wrote: > >> > >> Hi Alex, > >> > >> "IMO, it makes debugging the framework really painful." > >> > >> I feel your pain. I didn't try switching it off yet, but I > wonder if > >> it can > >> be on by default with a different approach: > >> example: > >> > >> > >> > this.org_apache_royale_jewel_beads_models_DropDownListModel__selectedIndex > >> > >> 1. have the 'private' part of the name as a suffix instead of > prefix. > >> It > >> means you can read the important part easier (doesn't help with > the > >> very > >> long lines with multiple references on them, but might be easier > in > >> general) > >> > >> 2. maybe the class name approach is not needed? > >> the problem domain is more about conflicts in inheritance chain > rather > >> than > >> class naming, is it not? > >> so if > >> -local private name is __selectedIndex > >> -and the current class is 5 levels away from Object base class > >> would it not be possible to use t > >> this.__selectedIndex_$P5 > >> or similar? > >> > >> if this 2nd part makes sense (still pondering it) then it could > >> probably be > >> on by default. > >> > >> > >> > >> On Thu, Nov 1, 2018 at 5:21 AM Alex Harui > <[email protected]> > >> wrote: > >> > >>> Hi Kenny, > >>> > >>> I went and made the changes to decorate private variable names. I > >> made it > >>> an option that's on by default. IMO, it makes debugging the > >> framework > >>> really painful. I'm probably going to turn the flag off in the > >> framework > >>> and have a policy that we don't use the same private variable > >> names. It > >>> shouldn't affect what users do. > >>> > >>> Also, turning the option off revealed a fair amount of duplicated > >> code. > >>> Sometimes we copy a file to form the basis of a subclass and then > >> don't > >>> clean up everything and you don't find out about private methods if > >> they > >>> have their names decorated. > >>> > >>> -Alex > >>> > >>> On 10/28/18, 5:22 AM, "Kenny Lerma" <[email protected]> wrote: > >>> > >>> Alex, this is absolutely needed. The same private variables > >> names in > >>> base > >>> class and subclass are quite common. Since this only affects > >> debug, > >>> there > >>> is no harm in the decorator and maintains consistency with > flash. > >>> > >>> > >>> On Sat, Oct 27, 2018, 9:05 PM Alex Harui > >> <[email protected]> > >>> wrote: > >>> > >>>> Hi, > >>>> > >>>> It appears that in Flash, private variables are actually in a > >> custom > >>>> namespace. This means you can have private APIs in a base > >> class and > >>>> private APIs in a subclass with the same name (and aren't > >> overrides) > >>> and > >>>> everything "works". IOW: > >>>> > >>>> package org.apache.royale.core { > >>>> public class BaseClass { > >>>> private var foo:String = "foo,"; > >>>> public function BaseClass() { > >>>> trace(foo); > >>>> } > >>>> }}} > >>>> > >>>> package org.apache.royale.core { > >>>> public class MyClass { > >>>> private var foo:String = "bar!"; > >>>> public function MyClass() { > >>>> super(); > >>>> trace(foo); > >>>> } > >>>> }}} > >>>> > >>>> var baz:MyClass = new MyClass(); // outputs foo,bar! > >>>> > >>>> This is true for private functions and getters/setters as well. > >>> However, > >>>> this currently does not work in JS, since the transpiled code > >> looks > >>> like: > >>>> > >>>> org.apache.royale.core.BaseClass = function() { > >> trace(this.foo) } > >>>> org.apache.royale.core BaseClass.prototype.foo = "foo,"; > >>>> > >>>> And > >>>> > >>>> org.apache.royale.core MyClass = function() { trace(this.foo} } > >>>> org.apache.royale.core MyClass.prototype.foo = "bar!"; > >>>> > >>>> So you will get "bar!bar!"; > >>>> > >>>> > >>>> The MX Charts code uses the same private API names in base > >> classes > >>> and > >>>> subclasses. I don't know why they didn't use protected > >> methods and > >>>> override them, so I'm going to change the Charts code to use > >>> overrides just > >>>> to keep making progress. > >>>> > >>>> I'm wondering if anybody else uses the same private API name > >> in base > >>>> classes and subclasses. The theoretical fix is to have the > >> compiler > >>>> generate a decorated name. That's what the compiler already > >> does for > >>>> mx_internal APIs and other custom namespace APIs, but I think > >> it > >>> would make > >>>> our code fatter and uglier to decorate private API names, so > >> I'm > >>> tempted to > >>>> have the compiler emit an error or warning instead. > >>>> > >>>> In order to guarantee uniqueness, we'd have to decorate with > >> the > >>> fully > >>>> qualified name of the class. Then the transpiled code would > >> look > >>> like: > >>>> > >>>> org.apache.royale.core.BaseClass = function() { > >>>> trace(this.org_apache_royale_core_BaseClass_foo) } > >>>> org.apache.royale.core > >>>> BaseClass.prototype.org_apache_royale_core_BaseClass_foo = > >> "foo,"; > >>>> > >>>> And > >>>> > >>>> org.apache.royale.core MyClass = function() { > >>>> trace(this.org_apache_royale_core_MyClass_foo} } > >>>> org.apache.royale.core > >>>> MyClass.prototype.org_apache_royale_core_MyClass_foo = "bar!"; > >>>> > >>>> IMO, that's a painful change to the transpiler, so I want to > >> make > >>> sure we > >>>> really need to do this. I think it won't impact minified > >> size, but > >>> it will > >>>> be noticeable when debugging the JS. > >>>> > >>>> Thoughts? > >>>> -Alex > >>>> > >>>> > >>>> > >>> > >>> > >>> > >> > >> > >> > > > > > > > >
