jyapayne/swig
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve correctness of SwigDerivedClassHasMethod() by making sure only methods that have override are used connected by director.

Browse source 
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();
  }
}
```
This commit is contained in:
Rokas Kupstys 2018-09-06 14:21:54 +03:00
commit 82a6fe02e3
1 changed files with 1 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Source/Modules/csharp.cxx
View file

@ -1953,7 +1953,7 @@ public:
Printf(proxy_class_code, " private bool SwigDerivedClassHasMethod(string methodName, global::System.Type[] methodTypes) {\n");
Printf(proxy_class_code,
" 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");
Printf(proxy_class_code, " bool hasDerivedMethod = methodInfo.DeclaringType.IsSubclassOf(typeof(%s));\n", proxy_class_name);
Printf(proxy_class_code, " bool hasDerivedMethod = methodInfo.IsVirtual && methodInfo.DeclaringType.IsSubclassOf(typeof(%s)) && methodInfo.DeclaringType != methodInfo.GetBaseDefinition().DeclaringType;\n", proxy_class_name);
/* Could add this code to cover corner case where the GetMethod() returns a method which allows type
* promotion, eg it will return foo(double), if looking for foo(int).
if (hasDerivedMethod) {