Skip to content

Commit

Permalink
[generator] Fix an issue where we'd not copy attributes from inlined …
Browse files Browse the repository at this point in the history
…protocols. (#17381)

Protocols with one set of introduced attributes ([TV (12, 0)]) inlined in
types that were introduced in a different version ([TV (10, 0)]) would always
use the attributes from the type.

This is wrong if the protocol was introduced after the type, in which case we
should instead use the introduced attributes from the protocol.

Fix this by choosing the latest introduced attribute when we have multiple to
choose from.

This required passing a bit more information around so that we always know if
a member is being inlined in another type.

This PR will also print availability attributes on the protocol members themselves:

	[Protocol]
	interface IProtocol
	{
		[TV (12, 0)] // this was not printed before
		[Export ("someProperty")]
		string SomeProperty { get; set; }
	}

Also add and improve some tests.

Contributes towards #14802.
  • Loading branch information
rolfbjarne authored Jan 30, 2023
1 parent 51bee95 commit 50c01fe
Show file tree
Hide file tree
Showing 7 changed files with 381 additions and 29 deletions.
7 changes: 7 additions & 0 deletions src/btouch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@

public class BindingTouch : IDisposable {
TargetFramework? target_framework;
#if NET
public static ApplePlatform [] AllPlatforms = new ApplePlatform [] { ApplePlatform.iOS, ApplePlatform.MacOSX, ApplePlatform.TVOS, ApplePlatform.MacCatalyst };
public static PlatformName [] AllPlatformNames = new PlatformName [] { PlatformName.iOS, PlatformName.MacOSX, PlatformName.TvOS, PlatformName.MacCatalyst };
#else
public static ApplePlatform [] AllPlatforms = new ApplePlatform [] { ApplePlatform.iOS, ApplePlatform.MacOSX, ApplePlatform.TVOS, ApplePlatform.WatchOS };
public static PlatformName [] AllPlatformNames = new PlatformName [] { PlatformName.iOS, PlatformName.MacOSX, PlatformName.TvOS, PlatformName.WatchOS };
#endif
public PlatformName CurrentPlatform;
public bool BindThirdPartyLibrary = true;
public bool skipSystemDrawing;
Expand Down
47 changes: 40 additions & 7 deletions src/generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3634,6 +3634,36 @@ static void CopyValidAttributes (List<AvailabilityBaseAttribute> dest, IEnumerab
}
}

// Find the introduced attribute with the highest version between the target list and the additions.
// If the destination list has an introduced attribute, replace it if it's not the one with the highest version
// If the destination list does not have an introduced attribute, then add one if there's one in the additions and there's not already an unavailable attribute.
static void FindHighestIntroducedAttributes (List<AvailabilityBaseAttribute> dest, IEnumerable<AvailabilityBaseAttribute> additions)
{
if (!additions.Any ())
return;

foreach (var platform in BindingTouch.AllPlatformNames) {
// find the availability attribute with the highest version we're trying to add
var latestAddition = additions
.Where (v => v.AvailabilityKind == AvailabilityKind.Introduced && v.Platform == platform)
.OrderBy (v => v.Version)
.LastOrDefault ();
if (latestAddition is null)
continue;

var added = CloneFromOtherPlatform (latestAddition, latestAddition.Platform);
var idx = dest.FindIndex (v => v.Platform == platform && v.AvailabilityKind == AvailabilityKind.Introduced);
if (idx == -1) {
// no existing introduced attribute: add it unless there's already an unavailable attribute
if (!dest.Any (v => v.Platform == platform && v.AvailabilityKind == AvailabilityKind.Unavailable))
dest.Add (added);
} else if (added.Version > dest [idx].Version) {
// replace any existing introduced attribute if the existing version is lower than the added one
dest [idx] = added;
}
}
}

// This assumes the compiler implements property methods as get_ or set_ prefixes
static PropertyInfo GetProperyFromGetSetMethod (MethodInfo method)
{
Expand Down Expand Up @@ -3705,6 +3735,7 @@ AvailabilityBaseAttribute [] GetPlatformAttributesToPrint (MemberInfo mi, Member
// Don't copy parent attributes if the conflict with the type we're inlining members into
// Example: don't copy Introduced on top of Unavailable.
CopyValidAttributes (availabilityToConsider, parentContextAvailability);
FindHighestIntroducedAttributes (availabilityToConsider, parentContextAvailability);
} else {
availabilityToConsider.AddRange (parentContextAvailability);
}
Expand Down Expand Up @@ -3741,7 +3772,7 @@ AvailabilityBaseAttribute [] GetPlatformAttributesToPrint (MemberInfo mi, Member
AddImpliedPlatforms (memberAvailability);

// Now copy it down introduced from the parent
CopyValidAttributes (memberAvailability, availabilityToConsider.Where (attr => attr.AvailabilityKind == AvailabilityKind.Introduced));
FindHighestIntroducedAttributes (memberAvailability, availabilityToConsider.Where (attr => attr.AvailabilityKind == AvailabilityKind.Introduced));

// Now expand the implied catalyst\TVOS from [iOS] a second time
// This is needed in some cases where the only iOS information is in the
Expand Down Expand Up @@ -5306,6 +5337,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
var minfo = new MemberInformation (this, this, pi, type, is_interface_impl);
bool use_underscore = minfo.is_unified_internal;
var mod = minfo.GetVisibility ();
Type inlinedType = pi.DeclaringType == type ? null : type;
minfo.protocolize = Protocolize (pi);
GetAccessorInfo (pi, out var getter, out var setter, out var generate_getter, out var generate_setter);

Expand Down Expand Up @@ -5493,7 +5525,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
PrintExport (minfo, sel, export.ArgumentSemantic);
}

PrintAttributes (pi.GetGetMethod (), platform: true, preserve: true, advice: true, notImplemented: true);
PrintAttributes (pi.GetGetMethod (), platform: true, preserve: true, advice: true, notImplemented: true, inlinedType: inlinedType);
#if NET
if (false) {
#else
Expand Down Expand Up @@ -5564,7 +5596,7 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
if (not_implemented_attr == null && (!minfo.is_sealed || !minfo.is_wrapper))
PrintExport (minfo, sel, export.ArgumentSemantic);

PrintAttributes (pi.GetSetMethod (), platform: true, preserve: true, advice: true, notImplemented: true);
PrintAttributes (pi.GetSetMethod (), platform: true, preserve: true, advice: true, notImplemented: true, inlinedType: inlinedType);
#if NET
if (false) {
#else
Expand Down Expand Up @@ -6408,6 +6440,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
minfo.is_export = true;

print ("[Preserve (Conditional = true)]");
PrintAttributes (pi, platform: true);

if (minfo.is_unsafe)
mod = "unsafe ";
Expand All @@ -6428,12 +6461,12 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
else
PrintExport (minfo, ea);
}
PrintAttributes (getMethod, notImplemented: true);
PrintAttributes (getMethod, notImplemented: true, platform: true);
print ("get;");
}
if (generate_setter) {
PrintBlockProxy (pi.PropertyType);
PrintAttributes (setMethod, notImplemented: true);
PrintAttributes (setMethod, notImplemented: true, platform: true);
if (!AttributeManager.HasAttribute<NotImplementedAttribute> (setMethod))
PrintExport (minfo, GetSetterExportAttribute (pi));
print ("set;");
Expand Down Expand Up @@ -6765,10 +6798,10 @@ public void PrintBindAsAttribute (ICustomAttributeProvider mi, StringBuilder sb
print (attribstr);
}

public void PrintAttributes (ICustomAttributeProvider mi, bool platform = false, bool preserve = false, bool advice = false, bool notImplemented = false, bool bindAs = false, bool requiresSuper = false)
public void PrintAttributes (ICustomAttributeProvider mi, bool platform = false, bool preserve = false, bool advice = false, bool notImplemented = false, bool bindAs = false, bool requiresSuper = false, Type inlinedType = null)
{
if (platform)
PrintPlatformAttributes (mi as MemberInfo);
PrintPlatformAttributes (mi as MemberInfo, inlinedType);
if (preserve)
PrintPreserveAttribute (mi);
if (advice)
Expand Down
3 changes: 3 additions & 0 deletions src/metal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1793,6 +1793,9 @@ interface IMTLTexture { }
partial interface MTLTexture : MTLResource {
[iOS (8, 0)]
[Deprecated (PlatformName.iOS, 10, 0)]
[Deprecated (PlatformName.MacOSX, 10, 12)]
[Deprecated (PlatformName.TvOS, 10, 0)]
[Deprecated (PlatformName.MacCatalyst, 13, 1)]
[Abstract, Export ("rootResource")]
IMTLResource RootResource { get; }

Expand Down
11 changes: 10 additions & 1 deletion tests/cecil-tests/ApiAvailabilityTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ public void FindMissingObsoleteAttributes ()
"MediaPlayer.MPVolumeSettings.AlertHide()",
"MediaPlayer.MPVolumeSettings.AlertIsVisible()",
"MediaPlayer.MPVolumeSettings.AlertShow()",
"Metal.IMTLResource Metal.MTLTextureWrapper::RootResource()",
"MetalPerformanceShaders.MPSCnnConvolutionDescriptor.GetConvolutionDescriptor(System.UIntPtr, System.UIntPtr, System.UIntPtr, System.UIntPtr, MetalPerformanceShaders.MPSCnnNeuron)",
"MetalPerformanceShaders.MPSCnnFullyConnected..ctor(Metal.IMTLDevice, MetalPerformanceShaders.MPSCnnConvolutionDescriptor, System.Single[], System.Single[], MetalPerformanceShaders.MPSCnnConvolutionFlags)",
"MetalPerformanceShaders.MPSCnnNeuron MetalPerformanceShaders.MPSCnnConvolution::Neuron()",
Expand Down Expand Up @@ -404,6 +403,16 @@ public void AttributeConsistency (AssemblyInfo info)
bool SkipSupportedAndObsoleteAtTheSameTime (ICustomAttributeProvider api, ApplePlatform platform, Version version)
{
var fullname = api.AsFullName ();

switch (fullname) {
case "SceneKit.SCNAnimationPlayer.SetSpeed(System.Runtime.InteropServices.NFloat, Foundation.NSString)":
// SetSpeed is in the SCNAnimatable protocol, which was added in iOS 8.0.
// The SetSpeed method was added in iOS 10.0, and deprecated in iOS 11.
// The SCNAnimatable protocol is implemented by the SCNAnimationPlayer class, which was added in iOS 11.
// Thus it's expected that the method was introduced and deprecated in the same OS version.
return true;
}

switch (platform) {
case ApplePlatform.iOS:
switch (fullname) {
Expand Down
10 changes: 10 additions & 0 deletions tests/cecil-tests/AttributeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,13 @@ static HashSet<string> IgnoreElementsThatDoNotExistInThatAssembly {
"Foundation.NSUserActivity.LoadDataAsync (System.String, Foundation.NSProgress&)",
"Foundation.NSUserActivity.WritableTypeIdentifiers",
"Foundation.NSUserActivity.WritableTypeIdentifiersForItemProvider",
"Foundation.NSUserActivity.get_WritableTypeIdentifiers ()",
"Foundation.NSUserActivity.get_WritableTypeIdentifiersForItemProvider ()",

// This is from the NSItemProviderReading protocol: NSUserActivity does not implement NSItemProviderReading on tvOS and macOS.
"Foundation.NSUserActivity.GetObject (Foundation.NSData, System.String, Foundation.NSError&)",
"Foundation.NSUserActivity.ReadableTypeIdentifiers",
"Foundation.NSUserActivity.get_ReadableTypeIdentifiers ()",

// This is from the NSItemProviderWriting protocol: MKMapItem does not implement NSItemProviderWriting on tvOS and macOS.
"MapKit.MKMapItem.GetItemProviderVisibilityForTypeIdentifier (System.String)",
Expand All @@ -282,14 +285,18 @@ static HashSet<string> IgnoreElementsThatDoNotExistInThatAssembly {
"MapKit.MKMapItem.LoadDataAsync (System.String, Foundation.NSProgress&)",
"MapKit.MKMapItem.WritableTypeIdentifiers",
"MapKit.MKMapItem.WritableTypeIdentifiersForItemProvider",
"MapKit.MKMapItem.get_WritableTypeIdentifiers ()",
"MapKit.MKMapItem.get_WritableTypeIdentifiersForItemProvider ()",

// This is from the NSItemProviderReading protocol: MKMapItem does not implement NSItemProviderReading on tvOS and macOS.
"MapKit.MKMapItem.GetObject (Foundation.NSData, System.String, Foundation.NSError&)",
"MapKit.MKMapItem.ReadableTypeIdentifiers",
"MapKit.MKMapItem.get_ReadableTypeIdentifiers ()",

// This is from the NSItemProviderReading protocol: PHLivePhoto does not implement NSItemProviderReading on tvOS and macOS.
"Photos.PHLivePhoto.GetObject (Foundation.NSData, System.String, Foundation.NSError&)",
"Photos.PHLivePhoto.ReadableTypeIdentifiers",
"Photos.PHLivePhoto.get_ReadableTypeIdentifiers ()",


// This is from the NSSecureCoding protocol: SKView only implements NSSecureCoding on macOS.
Expand All @@ -305,6 +312,9 @@ static HashSet<string> IgnoreElementsThatDoNotExistInThatAssembly {
"Metal.MTLTextureWrapper.FirstMipmapInTail",
"Metal.MTLTextureWrapper.IsSparse",
"Metal.MTLTextureWrapper.TailSizeInBytes",
"Metal.IMTLTexture.FirstMipmapInTail",
"Metal.IMTLTexture.IsSparse",
"Metal.IMTLTexture.TailSizeInBytes",


// HKSeriesBuilder doesn't implement the ISNCopying protocol on all platforms (and shouldn't on any according to the headers, so removed for XAMCORE_5_0).
Expand Down
Loading

4 comments on commit 50c01fe

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