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

[UIKit] Update bindings to Xcode 12 Beta 2 #8992

Merged
merged 10 commits into from
Jul 10, 2020

Conversation

dalexsoto
Copy link
Member

@dalexsoto dalexsoto commented Jul 2, 2020

We are still missing some APIs that can't be bound in this PR, the following task items are not necessarily needed to merge this PR but will need to be completed by Xcode 12 final:

  • Our generator cannot create block trampolines with generic NSObject subclasses. Filled issue [generator] Support for generic classes in our block trampolines #8993
    • Enable UICollectionViewDiffableDataSourceSectionSnapshotHandlers<ItemType> commented properties.
    • Enable UICollectionViewDiffableDataSourceReorderingHandlers<SectionType, ItemType> commented properties.
  • UTType from UniformTypeIdentifiers framework is not bound yet:
    • Enable UIDocumentPickerViewController constructors when UTType is bound.
    • Enable UIDocumentBrowserViewController.ctor when UTType is bound.
    • Enable UIDocumentBrowserViewController.ContentTypesForRecentDocuments when UTType is bound.
  • NSOrderedCollectionDifference from Foundation is not bound yet:
    • Enable NSDiffableDataSourceSectionTransaction<SectionIdentifierType, ItemIdentifierType>.Difference
    • Enable NSDiffableDataSourceTransaction<SectionIdentifierType, ItemIdentifierType>.Difference
  • Expose Intents framework to tvOS
    • Expose UIApplicationDelegate.GetHandlerForIntent to tvOS
  • [NoWatch, NoTV, NoiOS] ?
    • Check future availability on UICollectionView.ContextMenuInteraction [NoWatch, NoTV, NoiOS]
    • Check future availability on UISceneActivationRequestOptions.CollectionJoinBehavior [NoWatch, NoTV, NoiOS]

The above items are now part of #8943

@dalexsoto dalexsoto added this to the xcode12 milestone Jul 2, 2020
@dalexsoto dalexsoto requested a review from chamons as a code owner July 2, 2020 23:28
@spouliot spouliot mentioned this pull request Jul 2, 2020
46 tasks
Comment on lines 73 to 79
[MonoPInvokeCallback (typeof (DUICellAccessoryPosition))]
static unsafe nuint Invoke (IntPtr block, IntPtr accessories) {
var descriptor = (BlockLiteral *) block;
var del = (UICellAccessoryPosition) (descriptor->Target);
nuint retval = del (NSArray.ArrayFromHandle<UICellAccessory> (accessories));
return retval;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can use BlockLiteralRef and avoid it from being unsafe, something like:

static unsafe nuint Invoke (ref BlockLiteral block, IntPtr accessories) 

Comment on lines +86 to +89
public unsafe NIDUICellAccessoryPosition (BlockLiteral *block) : base (block)
{
invoker = block->GetDelegateForBlock<DUICellAccessoryPosition> ();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, can we use ref?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a copy/paste of generated code and, to remain optimizable, it should follow the exact same pattern

Comment on lines +97 to +98
var del = (UICellAccessoryPosition) GetExistingManagedDelegate (block);
return del ?? new NIDUICellAccessoryPosition ((BlockLiteral *) block).Invoke;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move unsafe inside with an unsafe block:

Suggested change
var del = (UICellAccessoryPosition) GetExistingManagedDelegate (block);
return del ?? new NIDUICellAccessoryPosition ((BlockLiteral *) block).Invoke;
var del = (UICellAccessoryPosition) GetExistingManagedDelegate (block);
unsafe {
return del ?? new NIDUICellAccessoryPosition ((BlockLiteral *) block).Invoke;
}

}

[BindingImpl (BindingImplOptions.Optimizable)]
unsafe nuint Invoke (UICellAccessory [] accessories)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In unsafe needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope sorry, missed that one cleaning the code produced by the generator

static internal readonly DUIConfigurationColorTransformerHandler Handler = Invoke;

[MonoPInvokeCallback (typeof (DUIConfigurationColorTransformerHandler))]
static unsafe IntPtr Invoke (IntPtr block, IntPtr color) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref BlockLiteral? We get unsafe out.

}

[BindingImpl (BindingImplOptions.Optimizable)]
unsafe UIColor Invoke (UIColor color)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the unsafe?

Comment on lines +10541 to +10549
NSLayoutConstraint ConstraintEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);

[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);

[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the naming.. but is so verbose! do we need, I'd do:

Suggested change
NSLayoutConstraint ConstraintEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
NSLayoutConstraint EqualToSystemSpaceAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint GreaterThanOrEqualToSystemSpaceAfterAncho (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint LessThanOrEqualToSystemSpaceAfterAncho (NSLayoutXAxisAnchor anchor, nfloat multiplier);

Copy link
Member Author

@dalexsoto dalexsoto Jul 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we can't do that, maybe for XAMCORE_4_0. This code exist on iOS too using the same names so we can increase code sharing between the platforms.

see:

xamarin-macios/src/uikit.cs

Lines 377 to 387 in 4ed6833

[TV (11,0), iOS (11,0)]
[Export ("constraintEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[TV (11,0), iOS (11,0)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);
[TV (11,0), iOS (11,0)]
[Export ("constraintLessThanOrEqualToSystemSpacingAfterAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingAfterAnchor (NSLayoutXAxisAnchor anchor, nfloat multiplier);

Comment on lines +10560 to +10571

[Mac (10,16)]
[Export ("constraintEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);

[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);

[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, super verbose:

Suggested change
[Mac (10,16)]
[Export ("constraintEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint EqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint GreaterThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[Mac (10,16)]
[Export ("constraintLessThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint LessThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same we can't :(

xamarin-macios/src/uikit.cs

Lines 401 to 411 in 4ed6833

[TV (11,0), iOS (11,0)]
[Export ("constraintEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[TV (11,0), iOS (11,0)]
[Export ("constraintGreaterThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintGreaterThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);
[TV (11,0), iOS (11,0)]
[Export ("constraintLessThanOrEqualToSystemSpacingBelowAnchor:multiplier:")]
NSLayoutConstraint ConstraintLessThanOrEqualToSystemSpacingBelowAnchor (NSLayoutYAxisAnchor anchor, nfloat multiplier);


[iOS (14,0)]
[Export ("updateVisibleMenuWithBlock:")]
void UpdateVisibleMenu (Func<UIMenu, UIMenu> handler);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use a delegate with names of what UIMenu first and UIMenu second are?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could but we gain not much from that, what you get in the parameter is the visible menu and the method is already called UpdateVisibleMenu and the return is well a menu too :/

[Export ("updateSearchResultsForSearchController:")]
void UpdateSearchResultsForSearchController (UISearchController searchController);
[Export ("updateSearchResultsForSearchController:")]
void UpdateSearchResultsForSearchController (UISearchController searchController);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need 'ForSearchController'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't change it, it would be a breaking change :) this is not new, looks like I fixed indentation

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

src/UIKit/UICellAccessory.cs Outdated Show resolved Hide resolved
src/UIKit/UICellAccessory.cs Outdated Show resolved Hide resolved
src/UIKit/UICellAccessory.cs Outdated Show resolved Hide resolved
src/UIKit/UICellAccessory.cs Outdated Show resolved Hide resolved
src/uikit.cs Show resolved Hide resolved
src/UIKit/UICellAccessory.cs Show resolved Hide resolved
[Watch (7,0), TV (14,0), iOS (14,0)]
[return: DelegateProxy (typeof (SDUICellAccessoryPosition))]
[BindingImpl (BindingImplOptions.Optimizable)]
public static UICellAccessoryPosition GetPositionBeforeAccessory (Type accessoryType) => GetPositionBeforeAccessory (new Class (accessoryType));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests.

src/UIKit/UICellAccessory.cs Show resolved Hide resolved
src/UIKit/UICellAccessory.cs Show resolved Hide resolved
internal delegate nuint DUICellAccessoryPosition (IntPtr block, IntPtr accessories);

//
// This class bridges native block invocations that call into C#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how complicated it would be to implement support for generating the supporting code for blocks by adding support in the generator for a [GenerateBlockCode] attribute to delegates:

[GenerateBlockCode]
delegate nuint UICellAccessoryPosition (UICellAccessory [] accessories);

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

src/UIKit/UICellAccessory.cs Outdated Show resolved Hide resolved
src/UIKit/UIEnums.cs Show resolved Hide resolved
@spouliot spouliot self-assigned this Jul 6, 2020
src/uikit.cs Outdated Show resolved Hide resolved
@monojenkins
Copy link
Collaborator

Build failure
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

7 tests failed, 86 tests passed.

Failed tests

  • xammac tests/Mac Modern/Debug: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Debug: BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): BuildFailure
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): BuildFailure

@spouliot
Copy link
Contributor

spouliot commented Jul 8, 2020

build

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

@spouliot spouliot changed the title [UIKit] Update bindings to Xcode 12 Beta 1 [UIKit] Update bindings to Xcode 12 Beta 2 Jul 9, 2020
@monojenkins
Copy link
Collaborator

Build failure
Build failed or was aborted

Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

Copy link
Member Author

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 thanks for taking this over @spouliot

@monojenkins
Copy link
Collaborator

Build failure
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

1 tests failed, 92 tests passed.

Failed tests

  • link all/iOS Unified 32-bits - simulator/Debug: Failed

@spouliot
Copy link
Contributor

spouliot commented Jul 9, 2020

test failure is a known, random issue -> https://github.com/xamarin/maccore/issues/1083

tests/monotouch-test/UIKit/CellAccessoryTest.cs Outdated Show resolved Hide resolved
src/uikit.cs Show resolved Hide resolved
@spouliot
Copy link
Contributor

Java NullPointerException in log :(

@spouliot
Copy link
Contributor

build

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

@spouliot spouliot merged commit 802bd4e into xamarin:xcode12 Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants