Skip to content

Commit

Permalink
Fix issue with properties implemented across multiple static interfac…
Browse files Browse the repository at this point in the history
…es (#1415)

* Fix issue where properties in static classes can be implemented across multiple different interfaces

* Add missed file
  • Loading branch information
manodasanW authored Dec 14, 2023
1 parent 4b40c13 commit 35d5018
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 23 deletions.
2 changes: 2 additions & 0 deletions src/Tests/TestComponentCSharp/TestComponentCSharp.idl
Original file line number Diff line number Diff line change
Expand Up @@ -505,11 +505,13 @@ namespace TestComponentCSharp
static void Method();
static Int32 Property;
static event Windows.Foundation.EventHandler<Int32> Event;
static Int32 ReadWriteProperty{ get; };
[contract(Windows.Foundation.UniversalApiContract, 10)]
{
static void WarningMethod();
static Int32 WarningProperty;
static event Windows.Foundation.EventHandler<Int32> WarningEvent;
static Int32 ReadWriteProperty{ set; };
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/Tests/TestComponentCSharp/WarningStatic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ namespace winrt::TestComponentCSharp::implementation
void WarningStatic::WarningEvent(winrt::event_token const& token) noexcept
{
}
int32_t WarningStatic::ReadWriteProperty()
{
return 0;
}
void WarningStatic::ReadWriteProperty(int32_t value)
{
}
}
2 changes: 2 additions & 0 deletions src/Tests/TestComponentCSharp/WarningStatic.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ namespace winrt::TestComponentCSharp::implementation
static void WarningProperty(int32_t value);
static winrt::event_token WarningEvent(Windows::Foundation::EventHandler<int32_t> const& handler);
static void WarningEvent(winrt::event_token const& token) noexcept;
static int32_t ReadWriteProperty();
static void ReadWriteProperty(int32_t value);
};
}
namespace winrt::TestComponentCSharp::factory_implementation
Expand Down
8 changes: 8 additions & 0 deletions src/Tests/UnitTest/TestComponentCSharp_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2660,6 +2660,14 @@ public void TestSetPropertyAcrossProjections()
Assert.Equal(4, property.ReadWriteProperty);
}

[Fact]
public void TestStaticPropertyImplementedAcrossInterfaces()
{
// Testing call doesn't fail.
WarningStatic.ReadWriteProperty = 4; // expected warning CA1416
_ = WarningStatic.ReadWriteProperty;
}

// Test scenario where type reported by runtimeclass name is not a valid type (i.e. internal type).
[Fact]
public void TestNonProjectedRuntimeClass()
Expand Down
108 changes: 85 additions & 23 deletions src/cswinrt/code_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2404,17 +2404,6 @@ Marshal.Release(inner);
std::nullopt);
}

void write_static_property(writer& w, Property const& prop, std::string_view prop_target, std::string_view platform_attribute = ""sv)
{
auto [getter, setter] = get_property_methods(prop);
auto getter_target = getter ? prop_target : "";
auto setter_target = setter ? prop_target : "";
write_property(w, prop.Name(), prop.Name(), write_prop_type(w, prop),
getter_target, setter_target, "public "sv, "static "sv, platform_attribute, platform_attribute,
!getter ? std::nullopt : std::optional(std::pair(prop.Parent(), prop)),
!setter ? std::nullopt : std::optional(std::pair(prop.Parent(), prop)));
}

void write_static_factory_event(writer& w, Event const& event, std::string_view event_target, std::string_view platform_attribute = ""sv)
{
write_event(w, event.Name(), event, event_target, "public "sv, ""sv, platform_attribute, std::nullopt);
Expand All @@ -2425,18 +2414,72 @@ Marshal.Release(inner);
write_event(w, event.Name(), event, event_target, "public "sv, "static "sv, platform_attribute, std::optional(std::tuple(event.Parent(), event, true)));
}

void write_static_members(writer& w, TypeDef const& static_type, TypeDef const& class_type)
void write_static_members(writer& w, TypeDef const& class_type)
{
auto vftblType = settings.netstandard_compat ?
w.write_temp("%.Vftbl", bind<write_type_name>(static_type, typedef_name_type::ABI, true)) :
"IUnknownVftbl";
write_static_objref_definition(w, vftblType, static_type, class_type);
auto cache_object = w.write_temp("%", bind<write_objref_type_name>(static_type));
std::map<std::string, std::tuple<std::string, std::string, std::string, std::string, std::string, std::optional<std::pair<TypeDef, Property>>, std::optional<std::pair<TypeDef, Property>>>> properties;

for (auto&& [interface_name, factory] : get_attributed_types(w, class_type))
{
if (factory.statics)
{
auto vftblType = settings.netstandard_compat ?
w.write_temp("%.Vftbl", bind<write_type_name>(factory.type, typedef_name_type::ABI, true)) :
"IUnknownVftbl";
write_static_objref_definition(w, vftblType, factory.type, class_type);
auto cache_object = w.write_temp("%", bind<write_objref_type_name>(factory.type));

auto platform_attribute = write_platform_attribute_temp(w, factory.type);
w.write_each<write_static_method>(factory.type.MethodList(), cache_object, platform_attribute);
w.write_each<write_static_event>(factory.type.EventList(), cache_object, platform_attribute);

// Merge property getters/setters, since such may be defined across interfaces
for (auto&& prop : factory.type.PropertyList())
{
auto [getter, setter] = get_property_methods(prop);
auto prop_type = write_prop_type(w, prop);

auto [prop_targets, inserted] = properties.try_emplace(std::string(prop.Name()),
prop_type,
getter ? cache_object : "",
getter ? platform_attribute : "",
setter ? cache_object : "",
setter ? platform_attribute : "",
!getter ? std::nullopt : std::optional(std::pair(prop.Parent(), prop)),
!setter ? std::nullopt : std::optional(std::pair(prop.Parent(), prop))
);
if (!inserted)
{
auto& [property_type, getter_target, getter_platform, setter_target, setter_platform, getter_prop, setter_prop] = prop_targets->second;
XLANG_ASSERT(property_type == prop_type);
if (getter)
{
XLANG_ASSERT(getter_target.empty());
getter_target = cache_object;
getter_platform = platform_attribute;
getter_prop = std::optional(std::pair(prop.Parent(), prop));
}
if (setter)
{
XLANG_ASSERT(setter_target.empty());
setter_target = cache_object;
setter_platform = platform_attribute;
setter_prop = std::optional(std::pair(prop.Parent(), prop));
}
XLANG_ASSERT(!getter_target.empty() || !setter_target.empty());
}
}
}
}

auto platform_attribute = write_platform_attribute_temp(w, static_type);
w.write_each<write_static_method>(static_type.MethodList(), cache_object, platform_attribute);
w.write_each<write_static_property>(static_type.PropertyList(), cache_object, platform_attribute);
w.write_each<write_static_event>(static_type.EventList(), cache_object, platform_attribute);
// Write properties with merged accessors
for (auto& [prop_name, prop_data] : properties)
{
auto& [prop_type, getter_target, getter_platform, setter_target, setter_platform, getter_prop, setter_prop] = prop_data;
write_property(w, prop_name, prop_name, prop_type,
getter_target, setter_target, "public "sv, "static "sv, getter_platform, setter_platform,
getter_prop,
setter_prop);
}
}

void write_attributed_types(writer& w, TypeDef const& type)
Expand Down Expand Up @@ -2489,10 +2532,10 @@ public static %I As<I>() => ActivationFactory.Get("%.%").AsInterface<I>();
type.TypeNamespace(),
type.TypeName());
}

write_static_members(w, factory.type, type);
}
}

write_static_members(w, type);
}

void write_nongeneric_enumerable_members(writer& w, std::string_view target)
Expand Down Expand Up @@ -3098,6 +3141,20 @@ remove => %.ErrorsChanged -= value;
return false;
};

std::function<bool(TypeDef const&)> search_interfaces_from_attributes = [&](TypeDef const& type)
{
for (auto&& [interface_name, factory] : get_attributed_types(w, type))
{
if (factory.statics && factory.type && (search_interface(factory.type) || search_interfaces(factory.type)))
{
return true;
}
}

return false;
};


// first search base interfaces for property getter
if (search_interfaces(setter_iface))
{
Expand All @@ -3116,6 +3173,11 @@ remove => %.ErrorsChanged -= value;
{
return { getter_iface, false };
}

if (search_interfaces_from_attributes(exclusive_to_type))
{
return { getter_iface, false };
}
}

throw_invalid("Could not find property getter interface");
Expand Down

0 comments on commit 35d5018

Please sign in to comment.