mono / CppSharp

Tools and libraries to glue C/C++ APIs to high-level languages
MIT License
3.12k stars 514 forks source link

Overriding of pure virtual method is incorrect in C# wrapper #1283

Open ArtsevDmitry opened 4 years ago

ArtsevDmitry commented 4 years ago

I'm playing around OpenCascade simple wrapper. I use one header "Geom2d_Line.hxx", class Geom2d_Line inherited from Geom2d_Curve which contain next method:

  Standard_EXPORT virtual Standard_Boolean IsPeriodic() const = 0;

in Geom2d_Line class this method overrides:

  Standard_EXPORT Standard_Boolean IsPeriodic() const Standard_OVERRIDE;

OS: Windows

when it converted to C#, I have property in Geom2dCurve

public abstract bool IsPeriodic
{
      get;
}

but in Geom2dLine I have these two members:

        /// <summary>Returns False</summary>
        public virtual bool IsPeriodic()
        {
            var __slot = *(void**) ((IntPtr) __OriginalVTables[0] + 13 * 8);
            var ___IsPeriodicDelegate = (global::UniCad.Delegates.Func_bool_IntPtr) Marshal.GetDelegateForFunctionPointer(new IntPtr(__slot), typeof(global::UniCad.Delegates.Func_bool_IntPtr));
            var __ret = ___IsPeriodicDelegate((__Instance + __PointerAdjustment));
            return __ret;
        }

and this in same CS class:

public override bool IsPeriodic { get; }

There are two issues:

  1. We have property and method with same names
  2. Property not overrided correctly (as a minimum should return IsPeriodic() method, but issue 1 in this case)

What should I do in this case, can I solve it with custom pass or something? Thanks.

Used headers

Geom2d_Line.hxx

Used settings

Target: MSVC

tritao commented 4 years ago

Thanks for reporting the bug.

Maybe we should not generate properties for methods, when they are virtual.

As a workaround, you can disable the pass that generates properties out of getters (GetterSetterToPropertyPass).

ddobrev commented 4 years ago

@ArtsevDmitry the correct behaviour should be to only get an overridden property with the body of the generated method which in turn should be gone. Unfortunately, I couldn't reproduce it here. The macros in your test case might be somehow involved. I would advise you to try generation with gradually removed macros to see if you can get to an even simpler test case.

angryzor commented 3 months ago

I ran into this bug as well and made a minimal reproduction:

namespace foo {
class Dorld {
    virtual int UnkFunc1() = 0;
};
}

class Bar : public foo::Dorld {
public:
    virtual int UnkFunc1() override;
};

Results in this output: https://gist.github.com/angryzor/5f00133280bce5e0cd523eb0d2562428

Note that the namespace is important. If Bar is declared inside the same namespace the bug is not triggered. However, my original code where I encountered this bug did have both classes in the same namespace, though they were in different header files.

[EDIT] I tried debugging the issue myself but I've been unsuccessful at setting up a working dev environment, I'm sorry :(

angryzor commented 3 months ago

I had another look and concluded that the bug occurs in GetterSetterToPropertyPass: it tries to determine whether the property overrides a base property here.

When Bar is outside the namespace Bar gets processed before Dorld, so Dorld doesn't have the property created yet. The check fails and the property is not created on Bar. The property is then later created on Dorld when the pass finally visits Dorld.

When instead Bar is inside the namespace, Dorld is visited first and the code continues as expected.

Looking to see if I can make a patch because this bug is really causing issues for me.

Ok yeah this is really simple, just need to add VisitFlags.ClassBases