NET Attribute Conversion] Generator PR cleanup #14802
Comments
chamons
added a commit
that referenced
this issue
May 2, 2022
…14779) This PR teaches our code generator to generate .NET 6 style availability attributes, adds a 4th Cecil test to verify our generated attributes, and a metric ton of API changes to satisfy that test. The generator work is the core of this PR, and includes: - Hacking out chunks of generator.cs that "helpfully" remove duplicate attributes, which are no longer duplicate in the new order that NET6 attributes force upon us. See changes in FilterMinimumVersion and PrintPlatformAttributes - Prevent a crash when the generator processes availability attributes with no version included (example: introduced on iOS but no version). See Is64BitiOSOnly. - The meat, GetPlatformAttributesToPrint, which synthesizes many attributes "out of thing air" from: - The parent context - Implied introduced just because the class exists on a given framework at all - Implied Catalyst because iOS exists - A few cludgy hacks PrintPlatformAttributesNoDuplicates and GenerateProperty because the existing PrintPlatformAttributes did not pass down parent context down, and the refactor was dangerous/too time consuming given time pressure. There are two intended API changes introduced by the reviews in this PR: - GetCurrentInputDevice was obviously intended by availability attributes to exist on Catalyst but due to define confusion was excluded. It is an addition in Microsoft.MacCatalyst.dll only. - The NEAppRule constructors were mis-marked on platforms, and were showing up incorrectly on Mac/Catalyst. I corrected the Catalyst one unconditionally, as we have not shipped Catalyst yet, but Mac is only fixed in NET6. There is a lot of follow up work in #14802 to do to remove a number of hard coded test failures, but this should be almost all of the remaining work in NET6 attributes. 🤞 this doesn't break too much for future us. Co-authored-by: Rolf Bjarne Kvinge <[email protected]> Co-authored-by: Manuel de la Pena <[email protected]>
3 tasks
chamons
added a commit
to chamons/xamarin-macios
that referenced
this issue
May 5, 2022
- Fixes xamarin#14905 by skipping with a link to xamarin#14802
rolfbjarne
pushed a commit
that referenced
this issue
May 6, 2022
vs-mobiletools-engineering-service2
pushed a commit
to vs-mobiletools-engineering-service2/xamarin-macios
that referenced
this issue
May 23, 2022
- Fixes xamarin#14905 by skipping with a link to xamarin#14802
|
@chamons should we postpone this to .NET 7? |
|
Yes |
rolfbjarne
pushed a commit
that referenced
this issue
May 24, 2022
) - Fixes #14905 by skipping with a link to #14802 Backport of #14911 Co-authored-by: Chris Hamons <[email protected]>
rolfbjarne
added a commit
to rolfbjarne/xamarin-macios
that referenced
this issue
Jan 17, 2023
The ifdefs here were confusing, so I cleaned them up a bit. Note that No* attributes in enum members won't prevent the enum members from being generated, they'll just get an unavailable attribute, so adding No* attributes to an enum member is not a breaking change (which allows for some of this cleanup). Contributes towards xamarin#14802.
rolfbjarne
added a commit
that referenced
this issue
Jan 19, 2023
The ifdefs here were confusing, so I cleaned them up a bit. Note that No* attributes in enum members won't prevent the enum members from being generated, they'll just get an unavailable attribute, so adding No* attributes to an enum member is not a breaking change (which allows for some of this cleanup). Contributes towards #14802.
rolfbjarne
added a commit
to rolfbjarne/xamarin-macios
that referenced
this issue
Jan 26, 2023
…protocols.
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 xamarin#14802.
rolfbjarne
added a commit
that referenced
this issue
Jan 30, 2023
…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.
rolfbjarne
added a commit
to rolfbjarne/xamarin-macios
that referenced
this issue
Feb 1, 2023
Seems like this is fixed now.
rolfbjarne
added a commit
that referenced
this issue
Feb 2, 2023
|
I believe all the bugs have been fixed now, so I'm closing this. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
This is the follow up work required after #14779 lands
and knowing when we should be use what type as a parent context is a mess. I papered over it mostly in the first PR, but in some cases I guessed wrong and generated a lot of attributes that are correct but could be skipped.
The text was updated successfully, but these errors were encountered: