Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[linker] Remove more and earlier some attributes #14270

Closed

Conversation

spouliot
Copy link
Contributor

  • [Model] and [Native] are not needed at runtime if the dynamic
    registrar is removed

  • Removing (and storing) the attributes earlier make it's possible for
    them not to be marked. IOW the attribute type itself (and not just the
    decorations) can be removed.

--- /var/folders/w_/zfvfxj_x3nbd8wlj9p4ck3xc0000gn/T/appcompare/diffcustomtooloutput/Microsoft.iOS.dll.a.text	2022-02-28 10:57:57.000000000 -0500
+++ /var/folders/w_/zfvfxj_x3nbd8wlj9p4ck3xc0000gn/T/appcompare/diffcustomtooloutput/Microsoft.iOS.dll.b.text	2022-02-28 10:57:57.000000000 -0500
@@ -43,14 +43,6 @@
 		{
 		}
 	}
-	[AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface)]
-	public sealed class ModelAttribute : Attribute
-	{
-		[MethodImpl(8)]
-		public ModelAttribute()
-		{
-		}
-	}
 	public sealed class You_Should_Not_Call_base_In_This_Method : Exception
 	{
 		[MethodImpl(8)]
@@ -584,11 +576,6 @@
 				return (Type)/*Error near IL_0001: Stack underflow*/;
 			}
 		}
-
-		[MethodImpl(8)]
-		public ProtocolAttribute()
-		{
-		}
 	}
 	[AttributeUsage(AttributeTargets.Interface, AllowMultiple = true)]
 	public sealed class ProtocolMemberAttribute : Attribute
@@ -1248,7 +1235,6 @@
 		}
 	}
 	[Register("Microsoft_iOS__UIKit_UIApplicationDelegate", false)]
-	[Model]
 	public class UIApplicationDelegate : NSObject, INativeObject, IDisposable
 	{
 		[MethodImpl(8)]
@@ -1270,7 +1256,6 @@
 		}
 	}
 	[Flags]
-	[Native]
 	public enum UIControlState : ulong
 	{
 		Normal = 0x0uL,
@@ -2931,14 +2916,6 @@
 			return (NativeHandle)/*Error near IL_0001: Stack underflow*/;
 		}
 	}
-	[AttributeUsage(AttributeTargets.Enum)]
-	public sealed class NativeAttribute : Attribute
-	{
-		[MethodImpl(8)]
-		public NativeAttribute()
-		{
-		}
-	}
 	public struct NativeHandle : IEquatable<NativeHandle>
 	{
 		private readonly IntPtr handle;

* `[Model]` and `[Native]` are not needed at runtime if the dynamic
   registrar is removed

* Removing (and storing) the attributes earlier make it's possible for
  them not to be marked. IOW the attribute type itself (and not just the
  decorations) can be removed.

```diff
--- /var/folders/w_/zfvfxj_x3nbd8wlj9p4ck3xc0000gn/T/appcompare/diffcustomtooloutput/Microsoft.iOS.dll.a.text	2022-02-28 10:57:57.000000000 -0500
+++ /var/folders/w_/zfvfxj_x3nbd8wlj9p4ck3xc0000gn/T/appcompare/diffcustomtooloutput/Microsoft.iOS.dll.b.text	2022-02-28 10:57:57.000000000 -0500
@@ -43,14 +43,6 @@
 		{
 		}
 	}
-	[AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface)]
-	public sealed class ModelAttribute : Attribute
-	{
-		[MethodImpl(8)]
-		public ModelAttribute()
-		{
-		}
-	}
 	public sealed class You_Should_Not_Call_base_In_This_Method : Exception
 	{
 		[MethodImpl(8)]
@@ -584,11 +576,6 @@
 				return (Type)/*Error near IL_0001: Stack underflow*/;
 			}
 		}
-
-		[MethodImpl(8)]
-		public ProtocolAttribute()
-		{
-		}
 	}
 	[AttributeUsage(AttributeTargets.Interface, AllowMultiple = true)]
 	public sealed class ProtocolMemberAttribute : Attribute
@@ -1248,7 +1235,6 @@
 		}
 	}
 	[Register("Microsoft_iOS__UIKit_UIApplicationDelegate", false)]
-	[Model]
 	public class UIApplicationDelegate : NSObject, INativeObject, IDisposable
 	{
 		[MethodImpl(8)]
@@ -1270,7 +1256,6 @@
 		}
 	}
 	[Flags]
-	[Native]
 	public enum UIControlState : ulong
 	{
 		Normal = 0x0uL,
@@ -2931,14 +2916,6 @@
 			return (NativeHandle)/*Error near IL_0001: Stack underflow*/;
 		}
 	}
-	[AttributeUsage(AttributeTargets.Enum)]
-	public sealed class NativeAttribute : Attribute
-	{
-		[MethodImpl(8)]
-		public NativeAttribute()
-		{
-		}
-	}
 	public struct NativeHandle : IEquatable<NativeHandle>
 	{
 		private readonly IntPtr handle;
```
@mandel-macaque mandel-macaque added the community Community contribution ❤ label Feb 28, 2022
@mandel-macaque
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

11 tests failed, 135 tests passed.

Failed tests

  • introspection/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 44 Passed: 42 Inconclusive: 0 Failed: 1 Ignored: 1)
  • monotouch-test/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 2570 Passed: 2488 Inconclusive: 6 Failed: 1 Ignored: 81)
  • monotouch-test/Mac [dotnet]/Debug (static registrar) [dotnet]: Failed (Test run failed.
    Tests run: 2567 Passed: 2486 Inconclusive: 6 Failed: 1 Ignored: 80)
  • monotouch-test/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 2708 Passed: 2593 Inconclusive: 10 Failed: 1 Ignored: 114)
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations) [dotnet]: BuildFailure
  • monotouch-test/tvOS - simulator/Release (all optimizations) [dotnet]: BuildFailure
  • link all/Mac [dotnet]/Release [dotnet]: BuildFailure
  • xammac tests/Mac Modern/Debug: Failed (Test run failed.
    Tests run: 2640 Passed: 2555 Inconclusive: 10 Failed: 2 Ignored: 83)
  • xammac tests/Mac Modern/Release: Failed (Test run failed.
    Tests run: 2637 Passed: 2551 Inconclusive: 10 Failed: 2 Ignored: 84)
  • xammac tests/Mac Modern/Release (all optimizations): Failed (Test run failed.
    Tests run: 2637 Passed: 2551 Inconclusive: 10 Failed: 1 Ignored: 85)
  • DotNet tests: Failed (Execution failed with exit code 1)

Pipeline on Agent XAMBOT-1109.Monterey'
Merge f7186db into a812ce4

@rolfbjarne
Copy link
Member

A few tests fails to build with:

ILLink : MacOSX error IL7000: An error occured while executing the custom linker steps. Please review the build log for more information. [/Users/builder/azdo/_work/1/s/xamarin-macios/tests/xharness/tmp-test-dir/link all93/link all.csproj]
ILLINK : error MM2362: The linker step 'Registrar' failed during processing: Object reference not set to an instance of an object. [/Users/builder/azdo/_work/1/s/xamarin-macios/tests/xharness/tmp-test-dir/link all93/link all.csproj]

  • link all / Mac [dotnet] / Release: build-Mac-20220228_121731.txt.zip
  • monotouch-test / iOS Unified 64-bits - simulator / Release (all optimizations) [dotnet]
  • monotouch-test /tvOS - simulator / Release (all optimizations) [dotnet]

@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [CI Build] Windows Integration Tests failed ❌

❌ Failed ❌

Pipeline on Agent
Hash: 78d0647231710b956e73c8de1436d85a003af878 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻

All tests on macOS M1 - Mac Big Sur (11) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 78d0647231710b956e73c8de1436d85a003af878 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: simulator tests

0 tests crashed, 16 tests failed, 154 tests passed.

Failures

❌ dotnettests tests (iOS)

1 tests failed, 0 tests passed.
  • DotNet tests: Failed (Execution failed with exit code 1)

Html Report (VSDrops) Download

❌ dotnettests tests (tvOS)

1 tests failed, 0 tests passed.
  • DotNet tests: Failed (Execution failed with exit code 1)

Html Report (VSDrops) Download

❌ monotouch tests (iOS)

3 tests failed, 8 tests passed.
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (managed static registrar, all optimizations) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (NativeAOT, x64) [dotnet]: Failed

Html Report (VSDrops) Download

❌ monotouch tests (MacCatalyst)

4 tests failed, 3 tests passed.
  • monotouch-test/Mac Catalyst [dotnet]/Release (NativeAOT) [dotnet]: Failed (Test run failed.
    Tests run: 2965 Passed: 2770 Inconclusive: 10 Failed: 3 Ignored: 192)
  • monotouch-test/Mac Catalyst [dotnet]/Release (NativeAOT, x64) [dotnet]: Failed (Test run failed.
    Tests run: 2965 Passed: 2828 Inconclusive: 10 Failed: 3 Ignored: 134)
  • monotouch-test/Mac Catalyst [dotnet]/Release (all optimizations) [dotnet]: BuildFailure
  • monotouch-test/Mac Catalyst [dotnet]/Release (managed static registrar, all optimizations) [dotnet]: BuildFailure

Html Report (VSDrops) Download

❌ monotouch tests (macOS)

4 tests failed, 4 tests passed.
  • monotouch-test/Mac [dotnet]/Release (NativeAOT, x64) [dotnet]: Failed (Test run failed.
    Tests run: 2849 Passed: 2688 Inconclusive: 4 Failed: 3 Ignored: 158)
  • monotouch-test/Mac [dotnet]/Release (NativeAOT) [dotnet]: Failed (Test run failed.
    Tests run: 2849 Passed: 2688 Inconclusive: 4 Failed: 3 Ignored: 158)
  • monotouch-test/Mac [dotnet]/Release (all optimizations) [dotnet]: Failed (Test run failed.
    Tests run: 2850 Passed: 2699 Inconclusive: 4 Failed: 4 Ignored: 147)
  • monotouch-test/Mac [dotnet]/Release (managed static registrar, all optimizations) [dotnet]: Failed (Test run failed.
    Tests run: 2850 Passed: 2700 Inconclusive: 4 Failed: 3 Ignored: 147)

Html Report (VSDrops) Download

❌ monotouch tests (tvOS)

3 tests failed, 8 tests passed.
  • monotouch-test/tvOS - simulator/Release (all optimizations) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Release (managed static registrar, all optimizations) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Release (NativeAOT, x64) [dotnet]: Failed

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ install-source: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 7 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac-binding-project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch (watchOS): All 4 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 78d0647231710b956e73c8de1436d85a003af878 [PR build]

rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this pull request Feb 29, 2024
Removing (and storing) the attributes earlier make it's possible for them not
to be marked. IOW the attribute type itself (and not just the decorations) can
be removed.

Inspired by / extracted from xamarin#14270.
Copy link
Contributor

Hi @spouliot. Due to inactivity, we will close this pull request in 7 days.

@rolfbjarne
Copy link
Member

I've been looking into this, and it's a lot trickier than it looks for various reasons, so I'll be closing this.

There's a tentative follow-up in #20224.

@rolfbjarne rolfbjarne closed this Mar 18, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this pull request May 6, 2024
Removing (and storing) the attributes earlier make it's possible for them not
to be marked. IOW the attribute type itself (and not just the decorations) can
be removed.

Inspired by / extracted from xamarin#14270.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution ❤
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants