Skip to content

Conversation

@Roee-BY
Copy link

@Roee-BY Roee-BY commented Apr 11, 2022

the function invocation mechanism was choosing the first compatible func according the the args and available overloads but there was a bug since some types that are available in java are unified into one type in JS for example JS number can be int double float or long hence choosing the first might cause a problem (in my case an android app crashes due to the incorrect overload). instead of the current mechanism i have implemented a mechanism that goes over all of the overloads and if there is more then one matching overload it invokes the method throwIfDispatcherAmbiguous(dispatcher) which will alert the user about the multiple a available overloads and ask his to choose which one to use.

@Roee-BY Roee-BY changed the title *bug fix* wrong overload invocation of overloaded function *bug fix* wrong overload invocation of overloaded functions Apr 13, 2022
@oleavr
Copy link
Member

oleavr commented May 25, 2022

Thanks! As much as I'd like to merge this, this is unfortunately a breaking change and cannot be merged until a future major bump. (There are no doubt existing scripts that rely on the first matching overload being picked.)

I think most errors like this stem from this anti-pattern:

SomeClass.someMethod.implementation = function (a, b, c) {
  return this.someMethod(a, b, c);
};

Which really should have been written something like this instead:

const someMethod = SomeClass.someMethod.overload('int', 'int', 'int');
someMethod.implementation = function (a, b, c) {
  return someMethod.call(this, a, b, c);
};

(Or looping over the overloads property and hooking all of them the same way -- capturing each one in a closure so the hook inside it doesn't need to waste CPU time looping over the possible overloads.)

Roee-BY and others added 3 commits September 22, 2022 13:45
the function invocation mechanism was choosing the first compatible func according the the args and available overloads but there was a bug since some types that are available in java are unified into one type in JS for example JS number can be int double float or long hence choosing the first might cause a problem (in my case an android app crashes due to the incorrect overload). instead of the current mechanism i have implemented a mechanism that goes over all of the overloads and if there is more then one matching overload it invokes the method throwIfDispatcherAmbiguous(dispatcher) which will alert the user about the multiple a available overloads and ask his to choose which one to use.
@oleavr
Copy link
Member

oleavr commented Sep 22, 2022

Just took a stab at landing this, and realized that it breaks the test-suite. Looking at the failures, it's clear that this will break a lot of scripts out there. So I'm not sure this is a good idea, even for a major bump. Let's revisit this at a later point in case any new ideas come into mind.

@Vishalsng112
Copy link

Has this bug been resolved? I believe the bug persists as the application crashes in my case due to the invocation of the incorrect overloaded function. Kindly inform me if it has been addressed or if you have any suggestions for a resolution. I am willing to make modifications to my local system to ensure it functions properly. The progress of my project (research work) is currently on hold due to this issue.

@Roee-BY
Copy link
Author

Roee-BY commented Nov 19, 2023

Hi, I'm glad to see someone else is trying to tackle this issue.
Looking back at my time trying to solve this, I find it hard to believe that some have solved it since the problem stems from the difference in typing between Java and Javascript, in my case, it was between js's Number and Java's int long float and double.
The best solution I can come up with for the problem you are facing is to choose the specific overload or overloads you want to invoke manually. You can do this by either specifying the argument's types and their order or by writing code that will choose the right overload to invoke by looking at all of the possible overload's signatures.
I do think that there should be a warrning when a specific overload is chosen arbitrarily regardless of whether or not there are more than one possible match, which is why I suggested it in #281

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.

3 participants