HaxeFoundation / haxe

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

Omitting function body when overriding @:coreApi classes. #3285

Open nadako opened 10 years ago

nadako commented 10 years ago

While working on splitting haxe.io stuff, I once again came to think that it would be useful to be able to omit function body for non-extern @:coreApi classes which would mean that function body is copied from the common class.

For example:

Comon haxe.io.Input

class Input {
    public function readByte() : Int {
        throw "Not implemented";
    }

    public function readInt8() : Int {
        var n = readByte();
        if( n >= 128 )
            return n - 256;
        return n;
    }
}

Target-specific haxe.io.Input

@:coreApi class Input {
    public function readByte() : Int {
        return someImplementation();
    }

    public function readInt8() : Int; // use the same algorithm
}
ncannasse commented 10 years ago

Seems fine, but maybe tag with a metadata for more explicit behavior ?

Simn commented 10 years ago

I would like to see a more general solution to this issue which is not tied to @:coreApi. I came across the same thing when playing with type classes where I would have an interface like this:

interface Eq<T> {
    public function equals(a:T, b:T):Bool;
    public function equalsNot(a:T, b:T):Bool;
}

In Haskell you could now create a concrete type class such as ArrayEq by defining either equals or equalsNot, with equals having a standard implementation of !equalsNot and vice versa.

I realize "there's a macro for that", but if we go for some sort of field copying anyway we might as well do it properly and sell it as compile-time inheritance.

nadako commented 10 years ago

I'm not sure I follow...

Simn commented 10 years ago

You want to copy fields from base implementations depending on @:coreApi metadata. I want an independent solution for that so it can be used in user code.

nadako commented 10 years ago

Yeah, but I don't understand how should that be used. You posted an example with interface, does that mean adding some default implementation to the interface itself?

Simn commented 10 years ago

That's just because I needed multiple inheritance which we can't really do with classes, even at macro-level. But now that you mention it a single inheritance copying solution would still be too limiting for my use case, so it wouldn't really help much.

Anyway, I don't like our current @:coreApi mechanic because it's limited to the standard API and requires equal type paths. It would be much better if we had a nice compile-time inheritance mechanism, so a target-specific class could be defined to contain all the fields of some base class minus the ones it defines itself.

But this probably leads to traits and language extensions on a too large scale, so I'm not gonna interfere anymore with this particular issue.

nadako commented 10 years ago

I think @:coreApi will do fine for now. The two most significant problems i see with it right now is this issue (reusing common methods) and that it overrides whole modules instead of types (i.e. if we want to split haxe.CallStack, we should copy its inner enum StackItem everywhere, which sucks hard and is stopping me from splitting haxe.CallStack

nadako commented 10 years ago

Pls review #3327

Simn commented 8 years ago

I have implemented something on this branch: https://github.com/HaxeFoundation/haxe/tree/ast_source

It has two components:

  1. @:astSource: Adding this to a method causes the compiler to add the method expression as argument. It becomes @:astSource({ the expression }).
  2. @:useAstSource dot.path.To.field: This inserts the @:astSource expression of To.field in its place. Additionally, if @:coreApi is added (@:useAstSource @:coreApi dot.path.To.field) then the core API class field is used instead.

With this we can express @nadako's example like so:

@:coreApi
class Input {
    public function readByte():Int {
        return 0;
    }

    public function readInt8():Int @:useAstSource @:coreApi haxe.io.Input.readInt8;
}

Obviously this could be abbreviated further to make it more elegant.

nadako commented 8 years ago

I wonder to what extent this is actually useful, since we still have to manually provide everything that is required for resolving identifiers in copied untyped AST (imports/usings/fields/etc), I'm a bit concerned that this is too error-prone to be a proper language feature.

Simn commented 8 years ago

Users certainly have to be aware, but then the same is true for macros that return expressions that are retrieved from external sources.

ncannasse commented 8 years ago

I don't like it that much either, that requires to be able to know in advance which AST will have to be reused, which breaks the separation of implementation between the base api and the platform-specific one.

Simn commented 8 years ago

You suggested exactly that and I've implemented it accordingly.

ncannasse commented 8 years ago

My suggestion was to have this done automatically, maybe it's the case?

Simn commented 8 years ago

Not yet, but to do "something" automatically you have to implement "something" first. Also I'm not sure how to approach this, i.e. how to detect which classes are core classes. This is especially true for your ideas in #4066.

ncannasse commented 8 years ago

look at Typeload.load_core_class: we're loading core classes in a separate core context, which have -D coreApi defined

Simn commented 8 years ago

Yes I figured that out after mentioning it and added it accordingly. I'm a bit concerned that it's for the most part a pointless waste of memory, but then again it's hardly going to make any difference in the big picture.