From a54ab5c42a5aa6076b1b369b092049f4873eb9f0 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Tue, 11 Apr 2023 20:12:35 +0200 Subject: [PATCH] [tools/tests] Fix bug in 'link all' test and the resulting regression that showed up in code. (#18016) There's a 'link all' test that's verifying that the IntroducedAttribute is linked away. It does so by verifying that the linked app doesn't have a 'IntroducedAttribute' type - but the test was constructing the fully qualified type name to look for incorrectly: ObjCRuntime.IntroducedAttribute, , Microsoft.iOS Note the double comma: that meant we wouldn't find the type, even if it wasn't linked away. The fix is easy (use a single comma), with one caveat (don't use a constant string, because the linker sees the reference to "ObjCRuntime.IntroducedAttribute" and _helpfully_ preserves it, exactly what we don't want), but it revealed that the tested behavior regressed: a fully linked app wouldn't link away the IntroducedAttribute. So a fix is also needed: properly remove TVAttribute, WatchAttribute and MacCatalystAttribute, which are subclasses of IntroducedAttribute (and what would make the linker keep IntroducedAttribute). Interestingly this showed up because of a bug in the runtime, where parsing the invalid assembly name would now throw an exception (https://github.com/dotnet/runtime/issues/84118). --- tests/linker/ios/link all/LinkAllTest.cs | 5 +++-- tools/linker/CoreRemoveAttributes.cs | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/linker/ios/link all/LinkAllTest.cs b/tests/linker/ios/link all/LinkAllTest.cs index 608a598ea7..03c2e65dd6 100644 --- a/tests/linker/ios/link all/LinkAllTest.cs +++ b/tests/linker/ios/link all/LinkAllTest.cs @@ -275,8 +275,9 @@ namespace LinkAll { [ThreadSafe] public void RemovedAttributes () { - const string prefix = NamespacePrefix; - const string suffix = ", " + AssemblyName; + // Don't use constants here, because the linker can see what we're trying to do and keeps what we're verifying has been removed. + string prefix = NamespacePrefix; + string suffix = AssemblyName; // since we're linking the attributes will NOT be available - even if they are used #if !XAMCORE_3_0 diff --git a/tools/linker/CoreRemoveAttributes.cs b/tools/linker/CoreRemoveAttributes.cs index 02cbb86cbe..2412aa1ab0 100644 --- a/tools/linker/CoreRemoveAttributes.cs +++ b/tools/linker/CoreRemoveAttributes.cs @@ -49,6 +49,10 @@ namespace Xamarin.Linker { case "NoMacAttribute": case "NoTVAttribute": case "NoWatchAttribute": + // special subclasses of IntroducedAttribute + case "TVAttribute": + case "MacCatalystAttribute": + case "WatchAttribute": return attr_type.Namespace == Namespaces.ObjCRuntime; // special subclasses of IntroducedAttribute case "iOSAttribute": @@ -73,6 +77,10 @@ namespace Xamarin.Linker { case "AvailabilityBaseAttribute": // base type for IntroducedAttribute and DeprecatedAttribute (could be in user code) case "DeprecatedAttribute": case "IntroducedAttribute": + // they are subclasses of ObjCRuntime.IntroducedAttribute + case "TVAttribute": + case "MacCatalystAttribute": + case "WatchAttribute": LinkContext.StoreCustomAttribute (provider, attribute, "Availability"); break; case "AdoptsAttribute":