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

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

Open
ArtsevDmitry opened this issue Jan 12, 2020 · 4 comments
Open

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

ArtsevDmitry opened this issue Jan 12, 2020 · 4 comments
Labels

Comments

@ArtsevDmitry
Copy link

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
Copy link
Collaborator

tritao commented Jan 12, 2020

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).

@tritao tritao added the bug label Mar 31, 2020
@ddobrev
Copy link
Contributor

ddobrev commented Apr 13, 2020

@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
Copy link
Contributor

angryzor commented Jul 25, 2024

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
Copy link
Contributor

angryzor commented Jul 26, 2024

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

angryzor added a commit to angryzor/CppSharp that referenced this issue Jul 26, 2024
angryzor added a commit to angryzor/CppSharp that referenced this issue Jul 26, 2024
angryzor added a commit to angryzor/CppSharp that referenced this issue Jul 26, 2024
tritao pushed a commit that referenced this issue Jul 26, 2024
* Fixes issue #1283

* Add test for issue #1283

* Move test for issue #1283 to CSharp test suite

* Fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants