Skip to content

Commit 82a6fe0

Browse files
committed
Improve correctness of SwigDerivedClassHasMethod() by making sure only methods that have override are used connected by director.
C# does not treat `virtual` methods of child classes as overriding (ass opposed to c++). In order to override a method it must have `override` specified. Previous version of this method treated `virtual void foo()` or `void foo()` in a subclass as methods that override virtual method of parent class. This resulted in `SwigDirectorConnect()` creating delegates and connecting them to a native class. Presence of such methods caused needless roundtrip through managed layer while in the end correct native function was called. This is the old code flow: ```cpp void SwigDirector_MyClass::nonOverride() { if (!swig_callbacknonOverride) { // 0. swig_callbacknonOverride is set MyClass::nonOverride(); return; } else { swig_callbacknonOverride(); // 1. this is called because wrapper mistakenly assumes overriding } } SWIGEXPORT void SWIGSTDCALL CSharp_director_basicNamespace_MyClass_nonOverrideSwigExplicitMyClass(void * jarg1) { MyClass *arg1 = (MyClass *) 0 ; arg1 = (MyClass *)jarg1; (arg1)->MyClass::nonOverride(); // 5. Correct method called in the end } ``` ```cs private void SwigDirectornonOverride() { nonOverride(); // 2. } public virtual void nonOverride() { if (SwigDerivedClassHasMethod("nonOverride", swigMethodTypes4)) // 3. This returns `true` director_basicPINVOKE.MyClass_nonOverrideSwigExplicitMyClass(swigCPtr); // 4. Native method of director class called explicitly else director_basicPINVOKE.MyClass_nonOverride(swigCPtr); } ``` This is the new code flow: ```cpp void SwigDirector_MyClass::nonOverride() { if (!swig_callbacknonOverride) { // 0. swig_callbacknonOverride is not set MyClass::nonOverride(); // 1. Calls correct method immediately return; } else { swig_callbacknonOverride(); } } ```
1 parent b90e3ae commit 82a6fe0

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

Source/Modules/csharp.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1953,7 +1953,7 @@ class CSHARP:public Language {
19531953
Printf(proxy_class_code, " private bool SwigDerivedClassHasMethod(string methodName, global::System.Type[] methodTypes) {\n");
19541954
Printf(proxy_class_code,
19551955
" global::System.Reflection.MethodInfo methodInfo = this.GetType().GetMethod(methodName, global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Instance, null, methodTypes, null);\n");
1956-
Printf(proxy_class_code, " bool hasDerivedMethod = methodInfo.DeclaringType.IsSubclassOf(typeof(%s));\n", proxy_class_name);
1956+
Printf(proxy_class_code, " bool hasDerivedMethod = methodInfo.IsVirtual && methodInfo.DeclaringType.IsSubclassOf(typeof(%s)) && methodInfo.DeclaringType != methodInfo.GetBaseDefinition().DeclaringType;\n", proxy_class_name);
19571957
/* Could add this code to cover corner case where the GetMethod() returns a method which allows type
19581958
* promotion, eg it will return foo(double), if looking for foo(int).
19591959
if (hasDerivedMethod) {

0 commit comments

Comments
 (0)