microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 238 forks source link

Bug: Windows.Ui.Xaml.Controls.Maps.MapControl.Style has ambiguous overload #1287

Closed dlech closed 1 year ago

dlech commented 1 year ago

Version

2.0.220531.1

Summary

I'm working on the Python bindings for WinRT. I came across a compile error due to an ambiguous overload that can't be resolved.

The problem stems from Windows.Ui.Xaml.Controls.Maps.MapControl which defined a Style property that returns a Windows.Ui.Xaml.Controls.Maps.MapStyle while at the same time, it inherits from Windows.Ui.Xaml.FrameworkElement which also defines a Style property that returns a Windows.Ui.Xaml.Style.

From what I have read, the only way to disambiguate methods in C++ is by the parameters, but since neither of the property getters take parameters, I'm not sure how to disambiguate. It seems like this is a bug in the API since classes shouldn't be overloading properties like this (equivalent of new in C# where one class shadows a property of another class instead of overriding it).

Reproducible example

#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>
#include <winrt/Windows.UI.Xaml.Controls.Maps.h>

using namespace winrt;
using namespace Windows::Foundation;
using namespace Windows::UI::Xaml::Controls::Maps;

int main()
{
    init_apartment();
    MapControl mc{};
    auto style = mc.Style();
}

Expected behavior

This should compile without error like literally every other API in the Windows SDK (we literally compile them all for Python bindings :wink:).

Actual behavior

Compiling fails with the following error. The 4 overloads are the two getters and the two setters. The setters work because they take 1 arg each with a different type but the getters are ambiguous because they both take no args.

image

Additional comments

No response

sylveon commented 1 year ago

Out of curiosity, how does this behave in C#?

dlech commented 1 year ago

As I guessed, CsWinRT shows a warning about needing the new keyword. So I suspect it will work in C# because the language actually supports hiding inherited members and it is only a warning rather than an error.

image
JaiganeshKumaran commented 1 year ago

This is weird because with standard C++ inheritance, if you overload by return type in a derived class, you always get the derived class version when you have the derived type and it's not an error.

Example:

struct Base
{
    int get();
};

struct Derived : Base
{
    float get();
};

void test()
{
    Derived d;
    auto const fValue = d.get(); // fValue is a float.
    auto const iValue = static_cast<Base&>(d).get(); // iValue is a int.
}
sylveon commented 1 year ago

MapStyle does not derive from Style

dlech commented 1 year ago

Here is the generated class:

struct WINRT_IMPL_EMPTY_BASES MapControl : winrt::Windows::UI::Xaml::Controls::Maps::IMapControl,
        impl::base<MapControl, winrt::Windows::UI::Xaml::Controls::Control, winrt::Windows::UI::Xaml::FrameworkElement, winrt::Windows::UI::Xaml::UIElement, winrt::Windows::UI::Xaml::DependencyObject>,
        impl::require<MapControl, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl2, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl3, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl4, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl5, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl6, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl7, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl8, winrt::Windows::UI::Xaml::Controls::IControl, winrt::Windows::UI::Xaml::Controls::IControl2, winrt::Windows::UI::Xaml::Controls::IControl3, winrt::Windows::UI::Xaml::Controls::IControl4, winrt::Windows::UI::Xaml::Controls::IControl5, winrt::Windows::UI::Xaml::Controls::IControl7, winrt::Windows::UI::Xaml::Controls::IControlProtected, winrt::Windows::UI::Xaml::Controls::IControlOverrides, winrt::Windows::UI::Xaml::Controls::IControlOverrides6, winrt::Windows::UI::Xaml::IFrameworkElement, winrt::Windows::UI::Xaml::IFrameworkElement2, winrt::Windows::UI::Xaml::IFrameworkElement3, winrt::Windows::UI::Xaml::IFrameworkElement4, winrt::Windows::UI::Xaml::IFrameworkElement6, winrt::Windows::UI::Xaml::IFrameworkElement7, winrt::Windows::UI::Xaml::IFrameworkElementProtected7, winrt::Windows::UI::Xaml::IFrameworkElementOverrides, winrt::Windows::UI::Xaml::IFrameworkElementOverrides2, winrt::Windows::UI::Xaml::IUIElement, winrt::Windows::UI::Xaml::IUIElement2, winrt::Windows::UI::Xaml::IUIElement3, winrt::Windows::UI::Xaml::IUIElement4, winrt::Windows::UI::Xaml::IUIElement5, winrt::Windows::UI::Xaml::IUIElement7, winrt::Windows::UI::Xaml::IUIElement8, winrt::Windows::UI::Xaml::IUIElement9, winrt::Windows::UI::Xaml::IUIElement10, winrt::Windows::UI::Xaml::IUIElementOverrides, winrt::Windows::UI::Xaml::IUIElementOverrides7, winrt::Windows::UI::Xaml::IUIElementOverrides8, winrt::Windows::UI::Xaml::IUIElementOverrides9, winrt::Windows::UI::Composition::IAnimationObject, winrt::Windows::UI::Composition::IVisualElement, winrt::Windows::UI::Xaml::IDependencyObject, winrt::Windows::UI::Xaml::IDependencyObject2>
    {
        MapControl(std::nullptr_t) noexcept {}
        MapControl(void* ptr, take_ownership_from_abi_t) noexcept : winrt::Windows::UI::Xaml::Controls::Maps::IMapControl(ptr, take_ownership_from_abi) {}
        MapControl();
        using winrt::Windows::UI::Xaml::Controls::Maps::IMapControl::FindMapElementsAtOffset;
        using impl::consume_t<MapControl, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl5>::FindMapElementsAtOffset;
        using winrt::Windows::UI::Xaml::Controls::Maps::IMapControl::GetLocationFromOffset;
        using impl::consume_t<MapControl, winrt::Windows::UI::Xaml::Controls::Maps::IMapControl5>::GetLocationFromOffset;
        using winrt::Windows::UI::Xaml::Controls::Maps::IMapControl::Style;
        using impl::consume_t<MapControl, winrt::Windows::UI::Xaml::IFrameworkElement>::Style;
        [[nodiscard]] static auto CenterProperty();
        [[nodiscard]] static auto ChildrenProperty();
        [[nodiscard]] static auto ColorSchemeProperty();
        [[nodiscard]] static auto DesiredPitchProperty();
        [[nodiscard]] static auto HeadingProperty();
        [[nodiscard]] static auto LandmarksVisibleProperty();
        [[nodiscard]] static auto LoadingStatusProperty();
        [[nodiscard]] static auto MapServiceTokenProperty();
        [[nodiscard]] static auto PedestrianFeaturesVisibleProperty();
        [[nodiscard]] static auto PitchProperty();
        [[nodiscard]] static auto StyleProperty();
        [[nodiscard]] static auto TrafficFlowVisibleProperty();
        [[nodiscard]] static auto TransformOriginProperty();
        [[nodiscard]] static auto WatermarkModeProperty();
        [[nodiscard]] static auto ZoomLevelProperty();
        [[nodiscard]] static auto MapElementsProperty();
        [[nodiscard]] static auto RoutesProperty();
        [[nodiscard]] static auto TileSourcesProperty();
        [[nodiscard]] static auto LocationProperty();
        static auto GetLocation(winrt::Windows::UI::Xaml::DependencyObject const& element);
        static auto SetLocation(winrt::Windows::UI::Xaml::DependencyObject const& element, winrt::Windows::Devices::Geolocation::Geopoint const& value);
        [[nodiscard]] static auto NormalizedAnchorPointProperty();
        static auto GetNormalizedAnchorPoint(winrt::Windows::UI::Xaml::DependencyObject const& element);
        static auto SetNormalizedAnchorPoint(winrt::Windows::UI::Xaml::DependencyObject const& element, winrt::Windows::Foundation::Point const& value);
        [[nodiscard]] static auto BusinessLandmarksVisibleProperty();
        [[nodiscard]] static auto TransitFeaturesVisibleProperty();
        [[nodiscard]] static auto PanInteractionModeProperty();
        [[nodiscard]] static auto RotateInteractionModeProperty();
        [[nodiscard]] static auto TiltInteractionModeProperty();
        [[nodiscard]] static auto ZoomInteractionModeProperty();
        [[nodiscard]] static auto Is3DSupportedProperty();
        [[nodiscard]] static auto IsStreetsideSupportedProperty();
        [[nodiscard]] static auto SceneProperty();
        [[nodiscard]] static auto BusinessLandmarksEnabledProperty();
        [[nodiscard]] static auto TransitFeaturesEnabledProperty();
        [[nodiscard]] static auto MapProjectionProperty();
        [[nodiscard]] static auto StyleSheetProperty();
        [[nodiscard]] static auto ViewPaddingProperty();
        [[nodiscard]] static auto LayersProperty();
        [[nodiscard]] static auto RegionProperty();
        [[nodiscard]] static auto CanTiltDownProperty();
        [[nodiscard]] static auto CanTiltUpProperty();
        [[nodiscard]] static auto CanZoomInProperty();
        [[nodiscard]] static auto CanZoomOutProperty();
    };

If I delete the line:

        using impl::consume_t<MapControl, winrt::Windows::UI::Xaml::IFrameworkElement>::Style;

Then it compiles as expected.

This code is generated by the following, so I suppose that should give some hint as to how I can use the metadata to detect potential issues like this and work around it.

https://github.com/microsoft/cppwinrt/blob/9e89b5c9fe3bb14ec4842ff54ea444a26c4495b0/cppwinrt/code_writers.h#L2320-L2369

kennykerr commented 1 year ago

Hi David, this is indeed a known API design bug - you can of course work around it by casting/querying for a more specific interface. Ideally, we can come up with some static analysis tooling that would catch this sort of thing.

dlech commented 1 year ago

Just to be clear, the bug is that MapControl should not have reused the same property name because it violates the rules set forth by https://learn.microsoft.com/en-us/uwp/winrt-cref/winrt-type-system rather than the code generators being able to detect and handle such cases?

kennykerr commented 1 year ago

Right, the bug is in the API design. Languages/bindings should be able to handle it either way, but it may not always be very convenient (requiring an explicit cast). Some loosely typed languages, like JavaScript, may not be able to handle it all.