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

feat: Add support for methods with same name and different parameters #1013

Merged
merged 9 commits into from
Dec 21, 2018

Conversation

tdermendjiev
Copy link
Contributor

A single FFICall (JSFunction) now holds a reference to a bucket of functions instead of the 1:1 FFICall - ObjCMethod we used to have. Depending on the parameters passed to the call we now decide runtime which method to invoke.|

A question worth asking is won't it be better if we swap the names of FFICall and FunctionWrapper because it would make more sense for the FunctionWrapper to have a vector of FFICalls (each FFICall with it's cif) instead of the contrary.

PR Checklist

What is the current behavior?

In Objective-C two methods with the same name but with different set of parameters would have different selectors (e.g. "baseMethod" and "baseMethod:") but they will still have the same jsName (which we call from js - e.g. "baseMethod" and "baseMethod"). Because of the 1:1 mapping of the FFICall and the ObjC method we are able to call only one of the methods.

What is the new behavior?

A check for the parameters is added so we know exactly which method to call runtime.

Implements #877 .

src/NativeScript/Calling/FFICall.cpp Outdated Show resolved Hide resolved
src/NativeScript/Calling/FFICall.cpp Outdated Show resolved Hide resolved
src/NativeScript/Calling/FFICall.h Outdated Show resolved Hide resolved
src/NativeScript/ObjC/ObjCPrototype.mm Outdated Show resolved Hide resolved
src/NativeScript/ObjC/ObjCPrototype.mm Outdated Show resolved Hide resolved
src/NativeScript/ObjC/ObjCPrototype.mm Outdated Show resolved Hide resolved
tests/TestRunner/app/MethodCallsTests.js Outdated Show resolved Hide resolved
tests/TestRunner/app/index.js Outdated Show resolved Hide resolved
@Natalia-Hristova
Copy link
Contributor

test

@Natalia-Hristova
Copy link
Contributor

test --performance

src/NativeScript/Calling/FFICall.cpp Outdated Show resolved Hide resolved
src/NativeScript/Calling/FFICall.h Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Show resolved Hide resolved
@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/functions-overloading branch 4 times, most recently from 6200475 to bc8d308 Compare November 6, 2018 12:31
src/NativeScript/Calling/FunctionWrapper.cpp Outdated Show resolved Hide resolved
src/NativeScript/Calling/FunctionWrapper.cpp Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Calling/FFICall.h Outdated Show resolved Hide resolved
src/NativeScript/ObjC/Constructor/ObjCConstructorNative.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Outdated Show resolved Hide resolved
src/NativeScript/Metadata/Metadata.mm Show resolved Hide resolved
@tdermendjiev
Copy link
Contributor Author

test

@mbektchiev mbektchiev force-pushed the tdermendzhiev/functions-overloading branch 4 times, most recently from 73cc2bd to 8c8172f Compare December 5, 2018 14:04
@mbektchiev mbektchiev modified the milestones: 5.1.0, 5.2.0 Dec 10, 2018
@tdermendjiev tdermendjiev force-pushed the tdermendzhiev/functions-overloading branch from a0eb1bc to df6103a Compare December 18, 2018 09:53
*override subspaceFor() method from all JSInternalFunction subclasses (i.e. new FunctionWrapper and derived classes)
*resolve conflicts
@tdermendjiev
Copy link
Contributor Author

test

@tdermendjiev
Copy link
Contributor Author

test

@tdermendjiev
Copy link
Contributor Author

test

@tdermendjiev tdermendjiev merged commit 741db1e into master Dec 21, 2018
@mbektchiev mbektchiev deleted the tdermendzhiev/functions-overloading branch December 21, 2018 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants