Skip to content

Commit

Permalink
[bgen] Adjust logic to handle protocol-backed events. (#19689)
Browse files Browse the repository at this point in the history
Previously we used the presence of the [Protocolize] attribute to detect
Delegate properties returning protocols that should be specially handled if
the delegate in question supported events (in particular the Delegate property
would get additional code to ensure the developer didn't use both the event
pattern and the delegate pattern, because then one or the other wouldn't work
properly).

However, the presence of the [Protocolize] attribute does not take into
account that there's another way to declare that a Delegate property uses a
protocol: the property could use the protocol directly as the property type,
without a [Protocolize] attribute.

So change the logic to detect either the [Protocolize] attribute or an actual
protocol.

This is a slight breaking change in that we'll now add the special code to
more Delegate properties, and we'll throw an exception if the developer uses
both the event pattern and the delegate pattern in more cases. In theory this
should be a good thing (we're detecting more broken developer code), and if it
turns out to be an incorrect check, developers can opt out of this extra check
anyway.
  • Loading branch information
rolfbjarne committed Dec 21, 2023
1 parent 44a2da0 commit 2bd2338
Showing 1 changed file with 15 additions and 15 deletions.
30 changes: 15 additions & 15 deletions src/bgen/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4147,7 +4147,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
// generated delegate. We check CheckForEventAndDelegateMismatches global to disable the checks
if (!BindThirdPartyLibrary && pi.Name.StartsWith ("Weak", StringComparison.Ordinal)) {
string delName = pi.Name.Substring (4);
if (SafeIsProtocolizedEventBacked (delName, type))
if (SafeIsProtocolEventBacked (delName, type))
print ("\t{0}.EnsureDelegateAssignIsNotOverwritingInternalDelegate ({1}, value, {2});", CurrentPlatform.GetApplicationClassName (), string.IsNullOrEmpty (var_name) ? "null" : var_name, GetDelegateTypePropertyName (delName));
}

Expand Down Expand Up @@ -6120,7 +6120,7 @@ public void Generate (Type type)
// See xamcore/test/apitest/DerivedEventTest.cs for specific tests

// Does this delegate qualify for this treatment. We fall back on the old "wrong" codegen if not.
bool isProtocolizedEventBacked = IsProtocolizedEventBacked (delName, type);
bool isProtocolEventBacked = IsProtocolEventBacked (delName, type);

// The name of the the generated virtual property that specifies the type of the leaf most internal delegate
string delegateTypePropertyName = GetDelegateTypePropertyName (delName);
Expand All @@ -6136,7 +6136,7 @@ public void Generate (Type type)

bool hasKeepRefUntil = bta.KeepRefUntil is not null;

if (isProtocolizedEventBacked) {
if (isProtocolEventBacked) {
// The generated virtual type property and creation virtual method
string generatedTypeOverrideType = shouldOverride ? "override" : "virtual";
print ("internal {0} Type {1}", generatedTypeOverrideType, delegateTypePropertyName);
Expand All @@ -6151,15 +6151,15 @@ public void Generate (Type type)
}

if (!hasKeepRefUntil)
print ("{0}_{1} Ensure{1} ()", isProtocolizedEventBacked ? "internal " : "", dtype.Name);
print ("{0}_{1} Ensure{1} ()", isProtocolEventBacked ? "internal " : "", dtype.Name);
else {
print ("static System.Collections.ArrayList? instances;");
print ("{0}_{1} Ensure{1} (object oref)", isProtocolizedEventBacked ? "internal " : "", dtype.Name);
print ("{0}_{1} Ensure{1} (object oref)", isProtocolEventBacked ? "internal " : "", dtype.Name);
}

print ("{"); indent++;

if (isProtocolizedEventBacked) {
if (isProtocolEventBacked) {
// If our delegate not null and it isn't the same type as our property
// - We're in one of two cases: The user += an Event and then assigned their own delegate or the inverse
// - One of them isn't being called anymore no matter what. Throw an exception.
Expand Down Expand Up @@ -6202,7 +6202,7 @@ public void Generate (Type type)

print ("#pragma warning disable 672");
print ("[Register]");
if (isProtocolizedEventBacked)
if (isProtocolEventBacked)
print ("internal class _{0} : {1}I{2} {{ ", dtype.Name, shouldOverride ? "_" + interfaceName + ", " : "NSObject, ", dtype.Name);
else
print ("sealed class _{0} : {1} {{ ", dtype.Name, TypeManager.RenderType (dtype));
Expand All @@ -6216,7 +6216,7 @@ public void Generate (Type type)
print ("public _{0} () {{ IsDirectBinding = false; }}\n", dtype.Name);


string shouldOverrideDelegateString = isProtocolizedEventBacked ? "" : "override ";
string shouldOverrideDelegateString = isProtocolEventBacked ? "" : "override ";

string previous_miname = null;
int miname_count = 0;
Expand Down Expand Up @@ -6252,7 +6252,7 @@ public void Generate (Type type)
print ("internal {0}? {1};", Nomenclator.GetDelegateName (mi), miname);

print ("[Preserve (Conditional = true)]");
if (isProtocolizedEventBacked)
if (isProtocolEventBacked)
print ("[Export (\"{0}\")]", FindSelector (dtype, mi));

print ("public {0}{1} {2} ({3})", shouldOverrideDelegateString, TypeManager.RenderType (mi.ReturnType), mi.Name, RenderParameterDecl (pars));
Expand Down Expand Up @@ -6693,17 +6693,17 @@ static string GetDelegateTypePropertyName (string delName)
return "GetInternalEvent" + delName + "Type";
}

bool SafeIsProtocolizedEventBacked (string propertyName, Type type)
bool SafeIsProtocolEventBacked (string propertyName, Type type)
{
return CoreIsProtocolizedEventBacked (propertyName, type, false);
return CoreIsProtocolEventBacked (propertyName, type, false);
}

bool IsProtocolizedEventBacked (string propertyName, Type type)
bool IsProtocolEventBacked (string propertyName, Type type)
{
return CoreIsProtocolizedEventBacked (propertyName, type, true);
return CoreIsProtocolEventBacked (propertyName, type, true);
}

bool CoreIsProtocolizedEventBacked (string propertyName, Type type, bool shouldThrowOnNotFound)
bool CoreIsProtocolEventBacked (string propertyName, Type type, bool shouldThrowOnNotFound)
{
PropertyInfo pi = type.GetProperty (propertyName);
BaseTypeAttribute bta = ReflectionExtensions.GetBaseTypeAttribute (type, this);
Expand Down Expand Up @@ -6738,7 +6738,7 @@ bool CoreIsProtocolizedEventBacked (string propertyName, Type type, bool shouldT
return false;
}

return Protocolize (pi) && bta.Events is not null && bta.Events.Any (x => x.Name == pi.PropertyType.Name);
return (Protocolize (pi) || IsProtocolInterface (pi.DeclaringType)) && bta.Events is not null && bta.Events.Any (x => x.Name == pi.PropertyType.Name);
}

string FindSelector (Type type, MethodInfo mi)
Expand Down

7 comments on commit 2bd2338

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.