Fix issue #1283

[angryzor]

See #1283 .

GetterSetterToPropertyPass currently fails on the following code:

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

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

Resulting output

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.

The bug occurs when 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.

Fixed by setting the VisitFlags.ClassBases flag on the pass.

[angryzor]

@dotnet-policy-service agree

[tritao]

Thanks so much for looking into this, any chance you could add a test to the test suite?

[angryzor]

Thanks so much for looking into this, any chance you could add a test to the test suite?

Sure, here you go! (I hope I added it in the correct place 😅)

[tritao]

We usually have these in https://github.com/mono/CppSharp/blob/main/tests/dotnet/CSharp/CSharp.Tests.cs / https://github.com/mono/CppSharp/blob/main/tests/dotnet/CSharp/CSharp.h which actually does end-to-end testing, but I guess there is nothing inherently wrong with your approach, its just gives a bit less guarantees.

[angryzor]

Agh, I had it there first, then saw there was a suite for the passes. Alright, I'll move it over. The strange thing is though, this bug prevents compilation on my machine, so I expected TestUncompilableCode to fail when I added the fixture, but that didn't happen. because I didn't undo my fix first. Never mind!

[angryzor]

So with the test in the new position, the build just fails and the tests can't even be run when the issue is present:

1>------ Build started: Project: CSharp.CSharp, Configuration: Release x64 ------
1>Skipping analyzers to speed up the build. You can execute 'Build' or 'Rebuild' command to run analyzers.
1><...>\CSharp\CSharp.cs(37969,33,37969,72): error CS0534: 'TestOverrideOfPropertyInNamespacedClass' does not implement inherited abstract member 'HasOverridenPropertyInNamespacedClass.Property.get'
1>Done building project "CSharp.CSharp.csproj" -- FAILED.

Is that acceptable?

[tritao]

I think so, there seems to be some delay in running the macOS CI, but looks good otherwise.

[tritao]

The mac CI is still not running, but since its already passing on other platforms, I will just merge it. Again, thanks for the fix.

[angryzor]

The mac CI is still not running, but since its already passing on other platforms, I will just merge it. Again, thanks for the fix.

No problem!