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

Implement method overloading in GDScript #1571

Open
me2beats opened this issue Sep 26, 2020 · 10 comments
Open

Implement method overloading in GDScript #1571

me2beats opened this issue Sep 26, 2020 · 10 comments

Comments

@me2beats
Copy link

me2beats commented Sep 26, 2020

Describe the project you are working on:
gdscript plugins

Describe the problem or limitation you are having in your project:
I often need to create similar methods that do similar things (and even usually return the same type), but differ in arguments (their number or their types).

The problem is that the more subtle the difference between the two methods, the more difficult it is to come up with a clear and concise names for the methods.
In addition, it is highly likely that in this case the methods will contain unnecessary redundant information that could easily be obtained from the methods signatures.

On the other hand, creating a single method using optional parameters partially solves the problem, but this creates difficulties in further extending the method, forcing to create a confusing code with a large number of if statements, and ultimately degrade the readability and support of the method.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Implementing method overloading in gdscript would solve this problem in most cases

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Perhaps one of the most beautiful illustrations of how convenient method overloading is, is the Color() methods.
Color

Note how method overloading saved the engine developers from having to create methods like
hex2color()
int2color()
etc

so in gdscript, one could declare several methods with the same name but different parameters.
when trying to declare 2 methods with exactly the same signatures, an error would appear:

The function "x" with signature ... already exist in the class

and when calling such an overloaded method, one just needs to choose a suitable signature, that could pop up just like in the case of Color():
Color2

If this enhancement will not be used often, can it be worked around with a few lines of script?:
above I indicated 2 workarounds (one is to set appropriate method names and the other is to use "universal" methods - all in one - with optional parameters), this may be acceptable in small projects, but most likely will lead to difficulties in maintaining such code as the project grows

Is there a reason why this should be core and not an add-on in the asset library?:
I think this should be core feature as it's directly related to gdscript

@Calinou Calinou changed the title GDScript: implement methods overloading Implement method overloading in GDScript Sep 26, 2020
@Xrayez
Copy link
Contributor

Xrayez commented Sep 29, 2020

I don't know how easy it would be to implement such a feature without making the codebase more complex, but this proposal showcased constructor overloading specifically (in the screenshots).

I know that one can override _init(), but then again there's no core support for this. Like with Color, constructors are registered via C++ and cannot be overridden in GDScript. I know they do not extend Object, but we're talking about method overloading which apply to any data structure: #279.

See also #1522 where linked list and other similar core data structures could have ability to override constructors on the C++ level, currently not possible, or I simply haven't found a way to do so.

@me2beats
Copy link
Author

me2beats commented Feb 8, 2021

Another good case when method overloading could be useful:
#737 (comment)

@vnen
Copy link
Member

vnen commented Mar 2, 2021

The main problem is that this can significantly increase the call dispatch time, since we can't always know what function to call until it's the time to do so. Even if only the number of arguments were considered, this would incur a penalty on all function calls, even those that are not overloaded (which is the majority).

The available overloaded constructors are not the fastest thing either.

I can't think of a way to implement this without this performance penalty, so I prefer to not do it since this isn't strictly needed.

@me2beats
Copy link
Author

I can't think of a way to implement this without this performance penalty, so I prefer to not do it since this isn't strictly needed.

Is it possible for performance penalty to happen only for overloaded methods?

@PrinceMerluza
Copy link

The main problem is that this can significantly increase the call dispatch time, since we can't always know what function to call until it's the time to do so. Even if only the number of arguments were considered, this would incur a penalty on all function calls, even those that are not overloaded (which is the majority).

How does other languages do it or Is this caused by the dynamic type nature of gdscript?

@vnen
Copy link
Member

vnen commented Apr 6, 2021

How does other languages do it or Is this caused by the dynamic type nature of gdscript?

It's not really because of being dynamic. The GDScript characteristic that makes it differ from most other languages is that each script is compiled in complete isolation (and there's no JIT compiler either). So it's pretty much impossible to do this in GDScript without doing it at the moment of the call, since the script is not aware of the whole environment when compiling. Other languages either are aware of the whole environment at once (so they know which functions exist at compile time) or they use JIT to offset the performance penalty (or they don't allow overloading at all).

Static typing is optional and will remain so, and any feature that works with typing but not without it is just asking for user confusion, so I don't intend to allow that.

The overloaded constructors now (on Godot 4.0) just get faster if you use static typing since those can be solved at compile time, otherwise they still go through the slow path of checking arity and argument types. Those are not so bad because they are always known at compile time to be a builtin type constructor, even with dynamic typing, so they don't interfere with regular function calling in general.

OTOH, supporting variadic functions (#1034) is more feasible, so people could use that and manually handle dispatch for overloads. This is so far the only possibility I see.


If anyone has a solution I'm all ears, but until then I'll keep this on hold.

@AlfishSoftware
Copy link

AlfishSoftware commented Mar 21, 2023

Is it possible for performance penalty to happen only for overloaded methods?

I think it should be possible like this...

people could use that and manually handle dispatch for overloads. This is so far the only possibility I see.

A compiler can generate that dispatch better (or at least not worse) than people, no?
Why not just make the compiler generate implicit methods (only when there's more than 1 option)?

That common method (with the original name) handles dispatch for the overloads and "rename-mangles" (so-to-speak) the specific overloads. This can be generated even in isolation, without considering other scripts at all.

When static typing info is available (e.g. calling from same script) the call resolves to the specific method name at compile-time; otherwise you call the common method which will handle dispatching at runtime (and have the same performance penalty that it would have if a user were to manually make code for dispatching it).

For example, this code:

## Some documentation
func load_stuff(id: int):
    # lots of code, etc
    return load_by_id(id)
## Other documentation
func load_stuff(name: String):
    # more code, etc
    return load_by_name(name)

Would be compiled to something equivalent to a code like this (just an oversimplified example):

## Join documentations
func load_stuff(arg1):
    match typeof(arg1):
        TYPE_INT: return load_stuff@1(arg1)
        TYPE_STRING: return load_stuff@2(arg1)
        _: # default is error, except if an overload with only Variant (unspecified type) arguments exists
            assert(false, "No overload for method `load_stuff` found for the provided arguments.")
            exit(1) # to prevent unexpected behavior, "crashing" is better than returning null
            # or you could REQUIRE all overloaded methods to mark one overload as the "default"
func load_stuff@1(id: int):
    # lots of code, etc
    return load_by_id(id)
func load_stuff@2(name: String):
    # more code, etc
    return load_by_name(name)

So x.load_stuff("id") would call the unified method normally, but if the compiler has enough typing info to resolve it at compile time, the call would become specific as if it was like x.load_stuff@2("id").

Of course, most calls to overloaded methods would be from other scripts, so they'd have the performance penalty. To reduce it, I think it would require compilation being a 2-step process where the API of the script is resolved/updated first, then the code is compiled/linked with knowledge of external scripts (or at least the list of all overloaded method APIs), allowing calls to resolve to specific overloads in most cases, so the fallback above is used just as a last resort (e.g. in cases like x.load_stuff(mysteriousObj) or x.call("load_stuff", "id")).

@shin0kaze
Copy link

I like the idea of @AlfishSoftware especially if this feature comes with variadic functions!

For those who want overloading with 0 overhead (i.e. for math), there may be another solution:

  1. First mark the desired functions with a keyword like overload or alias.
  2. Then, when compiler process these functions, it add to each function some suffix, like load_stuff_xd4c and load_stuff_w74a.
  3. Then compiler matches functions calls and definitions based on their signature.

The one drawback I see, is: to calling these functions dynamically, we need to store a reference to them.

Also we can mark these functions with a keyword on call too, so engine will know, when it need to use dispatching. At least with this - existing not overloaded functions won't be slower.

Don't bite too hard, just a thought :)

@clankill3r
Copy link

I don't know if the following suggestion would simplify the implementation but I really like Odin's approach.

foo :: proc{
    foo_bar,
    foo_baz,
    foo_baz2,
    another_thing_entirely,
}

https://odin-lang.org/docs/faq/#why-does-odin-have-explicit-procedure-overloading-but-not-implicit-procedure-overloading

@Egogorka
Copy link

As @AlfishSoftware pointed out, you could make the compiler do the type dispatch by itself. Essentially it only comes with a cost to the performance, for the user - I cannot see how it would cause confusion, you directly say that you need this code for that type, and other code for another.

And as @shin0kaze pointed out that with overload keyword could be signaling for an overload of a function and via adding a suffix to specify what kind of signature it has (if I understood it correctly) (also, why do you need a keyword for specifying overload? For backwards compatibility, when there were functions that had types but do not need overhead?). Only thing required from such suffix is that the map from signature to suffix is bijective and does not depend on the context, so it can be used in different scripts. I do not know the details of how godot is made, but it seems the case that we need that behavior. I don't know if such map is possible, but I think others can help (or maybe not a suffix but something else)

But another problem if there is a type only known at the runtime, which could be possible (I think? I did not program in GDScript much...) - you cannot know what kind of function to point to.

I guess a rough solution would be a simple "if":

  1. if the type is known at compile time - try calling the specified function. Because the map from types to suffixes is bijective and does not depend on context, you can easily resolve which one. If none is present, use a general one (and if there is none as well - throw an error)
  2. if the type is not known - make a function that checks for type at runtime - like @AlfishSoftware presented (with changing load_stuff@1 to load_stuff_xd4c, or something with suffix stuff) and fallback to 1st option because type is now known in the generated function

This could be done for any signature, although I think it would be harder. And you could also add
3. if the function with the needed number of arguments not found - use the varargs function, otherwise error

The problem is the second step - it makes runtime type checks which is not good. I don't think there is a solution for that case, almost by definition - you need to check the type at runtime because you only know it in the runtime. And because function typing is optional - this situation would come up with high frequency.

Still though, such a feature could be possible. Maybe put a warning in a debugger that tells that function overloading may cause an overhead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: On Hold
Development

No branches or pull requests

10 participants