-
Notifications
You must be signed in to change notification settings - Fork 520
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
[.NET] Proof-of-concept of removing ObjCRuntime.nfloat in favor of System.Runtime.InteropServices.NFloat. #13845
Conversation
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View dotnet API diffView dotnet legacy API diffAPI Current PR diffView dotnet API diffView dotnet legacy API diffGenerator diffℹ️ Generator Diff (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results73 tests failed, 73 tests passed.Failed tests
Pipeline on Agent XAMBOT-1096.BigSur' |
New commits in migueldeicaza/MonoTouch.Dialog: * migueldeicaza/MonoTouch.Dialog@3b9f579 Update code to cope with nfloat removal. Diff: https://github.com/migueldeicaza/MonoTouch.Dialog/compare/e8e0abe586fbe9351520704d3ca9480dc34ee77b..3b9f579868d4e53aefc12c30da0a3c0944ebbd69 New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@b5429b9 Update code to cope with nfloat removal. Diff: https://github.com/spouliot/Touch.Unit/compare/d668e4d99924acd04010c096029a8d3c7aed7586..b5429b9401dc1261f45671a52b3a5e9001240423
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got about 1/2 through when you realized you asked me to review the wrong PR. :)
* Code that references the full typename, `System.nfloat` won't compile. | ||
|
||
Fix: use `ObjCRuntime.nfloat` instead. | ||
// TODO: explain more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder before this goes live ^
xamarin_NFloat_objc_msgSend_exception (id self, SEL sel, GCHandle *exception_gchandle) | ||
{ | ||
@try { | ||
#if defined(__i386__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect NET6 + 32-bit to be a thing?
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View dotnet API diffView dotnet legacy API diffView dotnet iOS-MacCatalayst API diff
API Current PR diffℹ️ API Diff (from PR only) (please review changes) View dotnet API diffView dotnet legacy API diffView dotnet iOS-MacCatalayst API diff
Generator diffℹ️ Generator Diff (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results3 tests failed, 143 tests passed.Failed tests
Pipeline on Agent XAMBOT-1097.BigSur' |
The final PR is up: #14197. |
** DRAFT PR ** PROCEED AT OWN RISK **
This is just to get some CI on this.