From 82a6fe02e3772e6f09097a221fbb5756b622318e Mon Sep 17 00:00:00 2001
From: Rokas Kupstys <rokups@zoho.com>
Date: Thu, 6 Sep 2018 14:21:54 +0300
Subject: [PATCH] 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();
  }
}
```
---
 Source/Modules/csharp.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Source/Modules/csharp.cxx b/Source/Modules/csharp.cxx
index bd00ffaf3..75ee34023 100644
--- a/Source/Modules/csharp.cxx
+++ b/Source/Modules/csharp.cxx
@@ -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) {