Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Overload Support #93

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

TheDrawingCoder-Gamer
Copy link

Adds full overload support by renaming functions and function calls when compiling

Rendered Proposal

@Simn
Copy link
Member

Simn commented Oct 30, 2021

The detailed design section is supposed to be detailed. It's fine to have an incomplete starting point and work open questions out with the community, but for something as complicated as full overload support this is way too little. I'll leave it open for now, but in this state it's not even worth discussing much.

Some starting points:

  1. What about overrides and variance?
  2. What about interfaces, especially those with type parameters?
  3. What about reflection and calls through Dynamic? Who handles the actual dispatch?

I've always been against this feature because in my estimation it comes with too many additional requirements for the run-time.

@hughsando
Copy link
Member

You can just specify that it is exactly the same as properties.

    • You override the set_ function just like add_
    • interfaces can't define and adder
  1. 'setProperty' was introduced and support was required by the runtime - i think this should be avoided
    Or like abstracts, which pretty much amounts to the same thing.
    I guess I'm saying is make it a compile-time thing - there is already a lot of precedents (static extension/abstract/property) which solves almost all these problems.

@TheDrawingCoder-Gamer
Copy link
Author

I do think it should be compile time and that was what I was intending. I don't even know how it would happen on runtime.

@TheDrawingCoder-Gamer
Copy link
Author

Now that I think about, maybe it's unwise to not have a "true" function. I'd say maybe we could make a function that uses optional parameters but that isn't fully cross compatible afaik.

@TheDrawingCoder-Gamer
Copy link
Author

How would variance affect things? Simply doing how it already works, by "downgrading" (taking a subclass and removing all its properties that make it different from its parent) should work.

@back2dos
Copy link
Member

The main problem is diamond inheritance via interfaces.

interface I1 {
  function method(a:A):Void;
}
interface I2 {
  function method(b:B):Void;
}
class X implements I1 implements I2 {}

X is an I1 and an I2 and methods on instances of X could be invoked through any super type. For most targets this poses a problem. An alternative would be to always make the signature part of the generated method name, but that seems costly. Also, it's not generally possible if the interface is extern.

@TheDrawingCoder-Gamer
Copy link
Author

TheDrawingCoder-Gamer commented Oct 30, 2021

Are you even allowed to have an two interfaces with functions of the same name? That seems silly.
My specification says interfaces can't have overload functions and thus this should error.

@Simn
Copy link
Member

Simn commented Oct 30, 2021

Well, a compile-time-only feature is a long way from the proposed "fully work between all languages". I can't say that it sounds very attractive considering the complications it comes with and the inconsistencies that will arise. You will have to decide to what extent you want to support dynamic dispatch. It might be feasible to simply disallow some things here, but then the proposal has to say so.

Basically, if you want to support override then it get complicated, and if you don't then it seems like a weak feature to me.

@TheDrawingCoder-Gamer
Copy link
Author

This is all confusing, this is my first proposal so bear with me.

What I want is java like overloading support, for all target langs, either by using their built in overloading or emulating it. This proposal is more concerned with emulation.

Fully work with all languages simply means it can compile to all languages. For languages that don't natively support overloading, like hashlink, it's emulated. This proposal says that overloaded functions are renamed based on their type signature, and how haxe determines what signature is wanted.

I think as much as possible should be supported, but I'm not very good at working this stuff out. All that was in my mind making this was classes. Interfaces probably should not support overloading. Dynamic typing should probably be disallowed in overloaded functions because it's not helpful at all. Haxe already has a system for monomorphs, so those shouldn't be a problem.

@Simn
Copy link
Member

Simn commented Oct 30, 2021

Here's an example to make this easier to follow:

class Parent<T> {
	public function new() { }

	public function test(t:T) {
		trace("Parent.test(T)");
	}
}

class Child extends Parent<String> {
	public override overload function test(s:String) {
		trace("Child.test(String)");
	}
}

function main() {
	var child = new Child();
	child.test("foo");

	var parent:Parent<String> = child;
	parent.test("foo");

	var dyn:Dynamic = child;
	dyn.test("foo");
}
  • Your proposal covers the first case, which is the obvious one.
  • The second one is the most interesting because it is properly typed, but relies on dynamic dispatch. Renaming alone isn't sufficient to make this work, it likely requires adapter functions on Child.
  • The third case is the one you want to disallow, as I understand it, although it's not clear what that means. Would this just silently fail at run-time? That's a rather substantial concession...

Note that all three of these work on targets with real overload support.

@TheDrawingCoder-Gamer
Copy link
Author

The third one is impossible to error at compile time because it's dynamic correct? If so, then failing at runtime is probably the only solution, or using the getProperty reflection in some way. The second one is interesting. I'm not super sure on what "adapter function" means or "dynamic dispatch", but that example should obviously trace Child.test(string). The third example parent is confusing because it downcasting doesn't seem correct. It would make sense that overloaded override functions over functions with type parameters would override if the runtime signatures (meaning the function with all its type parameters filled in) are the same, and overload if they are not.

@Simn
Copy link
Member

Simn commented Oct 30, 2021

Dynamic dispatch is the thing that causes the child class function to be called even though the call is made on a reference typed as the parent class. If you think about it you will realize that this cannot be done at compile-time, hence dynamic dispatch.

Regarding adapter functions: Try constructing how the second example looks with overload functions being renamed (in whichever way). I think you'll naturally end up with an adapter function to make it work.

@hughsando
Copy link
Member

Another option might be to come at it from a different direction - that is call the "pseudo constructor" (static function that returns an instance) whatever you like and add the meta ':op(new)' or similar.

function new(a:Int, b:Float) { ... }
@:op(new) public static function create(x:Int) return new Test(x*3, x*5);

var test = new Test(1);  // Generates Test.create(1);

Making new private here might also help clear up ambiguous matching.
This arrangement makes is quite clear how super etc work, and using the static function resolution logic, calling new Derived(1) here would not find the Base.create function so it would not be possible to construct a Derived this way.

But compare new Test(1) with Test.create(1) and you have replaced a dot with a space and keyword with an identifier which is a pretty neutral exchange.

Fore me, the more general ability to overload 'create' is more interesting. Restricting to static functions might be a way of dodging dynamic resolution issues initially.

@Simn
Copy link
Member

Simn commented Oct 31, 2021

Please note that this already works, relying on the extern inline:

class Test {
	function new(a:Int, b:Float) { }
	public overload extern inline static function create(x:Int) return new Test(x * 3, x * 5);
	public overload extern inline static function create(x:Int, y:Int) return new Test(x * 3, y * 5);
}

function main() {
	Test.create(1);
	Test.create(1, 2);
}

The only problem is that you need 5 modifiers...

I'm not strictly against extending this to non-inline static functions (and final methods?), but it seems like a bit of a weird middle-ground.

@hughsando
Copy link
Member

Hey, I did not know that! As you say, the "extern" seems not only redundant but contradictory given that you have a function body, and I assume there is no technical/practical requirement for inline for a static function since dynamic dispatch confusion is not possible? Seems like an unnecessary restriction - but I guess you could alway just inline a call to a non-inline function if you really wanted.
So maybe @:op(new) would be possible then - although only marginally better.

@Simn
Copy link
Member

Simn commented Oct 31, 2021

Actually, extern inline is just our way of saying "compile-time". The extern modifier has historically been used to express "this has to be inlined because it doesn't exist at run-time". This was from way back when inlining was canceled under various circumstances and I don't think it is strictly necessary anymore. However, it also takes care of the whole reflection situation because inline functions without it are still required to be callable through dynamic means at run-time.

If we go the renaming route here to actually generate these overloads, there would be increased cognitive burden because it adds more exceptions to which functions aren't callable at run-time. That's not a hard showstopper, but it does make me reluctant to extend support for this in some half-assed way...

@TheDrawingCoder-Gamer
Copy link
Author

TheDrawingCoder-Gamer commented Oct 31, 2021

So would the adapter function be the "correct" name?

// from child extending Parent<String>
override overload function test(i:String)
// from parent
function test<T>(i:T)

would become

// child
override function test(i:Dynamic) {
    // switch runtime type 
    switch (Type.typeof(i)) {
        // Or whatever the proper switch is. Since at compile time String == String this is the only case
        case TClass(String): 
            test_string(i);
      // if it wasn't same at compile time 
     // case TClass(String): 
      //      super.test(i);
    }
}
function test_string(i:String)
// from parent. we know however that T is string so this isn't used for dynamic dispatch
function test(i:T)

@TheDrawingCoder-Gamer
Copy link
Author

I don't like the idea of runtime type switching though.

@TheDrawingCoder-Gamer
Copy link
Author

It probably would make sense to always keep an adapter function on every class to support dynamic dispatch. However it would still probably be optimal to rename any function we know uses a specific overload (because that type switching is probably less performant than simply calling a function). This also would mess up inlining really badly. How the heck do you inline that switch case?

@Simn
Copy link
Member

Simn commented Oct 31, 2021

Such a function would likely be needed for the full dynamic route, yes. But that's actually one step ahead of the other problem (case 2 from my example). In that situation, Child would likely override a test_T function and call its own test_String function (with a cast). That's how the JVM target handles this as well, and I think I stole that from the Java compiler.

Another question will be how to design the naming function with regards to type parameters.

@TheDrawingCoder-Gamer
Copy link
Author

TheDrawingCoder-Gamer commented Oct 31, 2021

I think we should ditch the lowercasing. That just makes it easier to make mistakes, and it allows stuff like

overload function test(i:TestClass)
overload function test(i:Testclass) 

to become

function test_testclass(i:TestClass)
function test_testclass(i:Testclass)

which is bad. I also think we shouldn't add the return type if it doesn't matter (I stole the idea of it not mattering from java too.) I also just realized something. Without a fully qualified typepath names could be the same.
I.E.

overload function test(i:bulby.Vector3)
overload function test(i:peote.Vector3)

wouldn't work:

function test_Vector3(i:bulby.Vector3)
function test_Vector3(i:peote.Vector3)

It would be silly to put full type paths tho. For byte code targets (like hashlink and neko) we could just get away with assigning a unique identifier and calling it a day. For targets that must be readable then we could get away with only qualifying if there would be a conflict otherwise. Based off that earlier example:

function test_bulby_Vector3(i:bulby.Vector3)
function test_peote_Vector3(i:peote.Vector3)

I can see why this needs further explaining. Why would it need to override test_T however??? The parent technically doesn't overload so it shouldn't be renamed.

@TheDrawingCoder-Gamer
Copy link
Author

That makes me think, how would overriding an overloaded function work?

class Parent {
     overload function test(i:String)
     overload function test(i:Int)
}
class Child extends Parent {
    override overload function test(i:Vector3)
    override overload function test(i:String)
}

would become

class Parent {
    function test(i:Dynamic) {
        switch (Type.typeof(i)) {
            case TClass(String): 
                test_String(i);
            case TInt: 
                test_Int(i);

        }
    }
    function test_String(i:String)
    function test_Int(i:Int)
}

class Child extends Parent {
    override function test(i:Dynamic) {
        switch (Type.typeof(i)) {
            case TClass(Vector3): 
                test_Vector3(i);
            default: 
                super.test(i);
        }
    }
    override function test_String(i:String)
}

@Simn
Copy link
Member

Simn commented Oct 31, 2021

I can see why this needs further explaining. Why would it need to override test_T however??? The parent technically doesn't overload so it shouldn't be renamed.

Right, adding overload to Parent.test is a case to consider too though, although mainly due to the type parameter naming issue. There could probably even be cases where we need multiple adapter functions if the hierarchies get crazy enough with some stuff being overloaded and some not.

@TheDrawingCoder-Gamer
Copy link
Author

Yes, I just made a case for this.

@TheDrawingCoder-Gamer
Copy link
Author

I feel like technically this could be done with macros but it'd be a really janky, messy code solution.

@TheDrawingCoder-Gamer
Copy link
Author

I'll update the proposal tommorow to cover these edge cases.

@TheDrawingCoder-Gamer
Copy link
Author

Ok I'm updating it, but you know how constructors return nothing? How would I make an adapter function for that?

@TheDrawingCoder-Gamer
Copy link
Author

I've made the decision to rename the overload converted functions with a hx__ prefix

@back2dos
Copy link
Member

back2dos commented Nov 2, 2021

Are you even allowed to have an two interfaces with functions of the same name? That seems silly.

I'm not sure what's unclear here. The following compiles and runs with the expected output on java, jvm and cs targets:

function main() {
  var x = new X(),
      a = new A(),
      b = new B();

  (x:I1).method(a);
  (x:I2).method(b);
  x.method(a);
  x.method(b);
}

class A {
  public function new() {}
}
class B {
  public function new() {}
}
interface I1 {
  function method(a:A):Void;
}
interface I2 {
  function method(b:B):Void;
}

class X implements I1 implements I2 {
  public function new() {}
  overload public function method(a:A) { trace(a); }
  overload public function method(b:B) { trace(b); }
}

If you're intending to not support this case, then that may be a wise decision, but it still deserves mention (because "full" overload support suggests otherwise) and finally needs a good error message / documentation.

That said, I'm leaning towards believing that that could be handled by the "adapter method" that dispatches on type. I suspect though that on static platforms without overloads it means that the runtime type of I1::method and I2::method would be the equivalent of Dynamic->Void and that could incur overhead in classes that implement only one of both interfaces.

@TheDrawingCoder-Gamer
Copy link
Author

What I'm really worried about is if there are two functions overloaded together and one of them is void and another is some other random type
I'm sure the type checker will handle this but we'll have to return something.

@TheDrawingCoder-Gamer TheDrawingCoder-Gamer changed the title Full Overload Support Better Overload Support Nov 2, 2021
@TheDrawingCoder-Gamer
Copy link
Author

I'm also positive we could implement this, but I would have to figure out a way to write that into a proposal.

@skial skial mentioned this pull request Nov 3, 2021
1 task
@TheDrawingCoder-Gamer
Copy link
Author

I just noticed that haxe already knows how to mangle names; it does it with the @:generic meta tag. Maybe we could use something like that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants