Skip to content

Commit

Permalink
[tools/tests] Fix bug in 'link all' test and the resulting regression…
Browse files Browse the repository at this point in the history
… that showed up in code.

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
(dotnet/runtime#84118).
  • Loading branch information
rolfbjarne committed Apr 10, 2023
1 parent c351760 commit b474324
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
5 changes: 3 additions & 2 deletions tests/linker/ios/link all/LinkAllTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,9 @@ public void DetectPlatform ()
[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
Expand Down
8 changes: 8 additions & 0 deletions tools/linker/CoreRemoveAttributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ protected override bool IsRemovedAttribute (CustomAttribute attribute)
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":
Expand All @@ -73,6 +77,10 @@ protected override void WillRemoveAttribute (ICustomAttributeProvider provider,
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":
Expand Down

0 comments on commit b474324

Please sign in to comment.