HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.18k stars 655 forks source link

@:keep does not keep fields of parent class #4903

Closed andyli closed 8 years ago

andyli commented 8 years ago

I think @:keep should automatically keep all the fields of parent class. It is not the case currently and the problem is illustrated as follows (compiles with -dce full):

class Base {
    public function new():Void {}
    public function test():Void {}
}

@:keep class Sub extends Base {
    // `test` will be kept if we override it in `Sub`.
    // However, it would be impossible to override if it were declared it as inline in Base...
    // override public function test() super.test();
}

class Test {
    static function main():Void {
        var sub:Dynamic = new Sub();
        sub.test();
    }
}

PS. It is undesirable to modify the definition of Base since it could be defined in a 3rd party lib.

Simn commented 8 years ago

I don't know if we really want that...

andyli commented 8 years ago

If we don't (the current behaviour), it would make a difference between inherited fields and the non-inherited ones. It doesn't sound right OOP-wise.

Moreover, I'm not sure in what use-case a user would @:keep a class without the need of keeping the super classes' fields... I can think of one use case of needing to keep super classes' fields. It is providing APIs for hscript users.

Simn commented 8 years ago

If we don't (the current behaviour), it would make a difference between inherited fields and the non-inherited ones. It doesn't sound right OOP-wise.

I'm not sure I understand this point. The only difference should be that overridden fields are kept if any of their overrides is kept. This is just another instance of the field being "used".

Moreover, I'm not sure in what use-case a user would @:keep a class without the need of keeping the super classes' fields...

Something like @:keep class MyApp extends SomeHugeFrameworkApp { } comes to mind.

I can think of one use case of needing to keep super classes' fields. It is providing APIs for hscript users.

Can you be more specific, I would like to understand how this is a problem.

andyli commented 8 years ago

For providing APIs for hscript users, let's say we made a game-maker app, and want to let users to use hscript to create their game logic. We created a set of game framework classes subclassing HaxePunk's stuff. Users should be able to use any field no matter if it is inherited or not.

We can @:keep our own classes, but keeping HaxePunk's classes would be complicated. Use of --macro keep("com.haxepunk") is undesirable since it would keep too much. Use of --macro keep() for each individual classes in the inheritance chain is troublesome since we have to list out the classes, and the list have to be updated when HaxePunk is refactored (e.g. if an extra class is introduced in the chain, from B extends A to B extends A2 extends A).

ncannasse commented 8 years ago

That seems to be a quite particular behavior that should definitely be handled through macros, not by declaring @:keep subclasses. Remember that you can add metadata during compilation.

Simn commented 8 years ago

I was thinking to add @:keepParent, but that reminded me of #3959 and that I would really like to have @:keep.parent, @:keep.child and @:keep.init.