Bug 58792 - [generator] Disallow the use of [Async] when the signature contains ref/out parameters
Summary: [generator] Disallow the use of [Async] when the signature contains ref/out p...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Tools (show other bugs)
Version: XI 10.99 (xcode9)
Hardware: PC Mac OS
: Normal normal
Target Milestone: Future Cycle (TBD)
Assignee: Alex Soto [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2017-08-15 21:09 UTC by Mike Norman
Modified: 2017-12-18 14:21 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: No
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Mike Norman 2017-08-15 21:09:10 UTC
Actual:

In MonoTouch.UIKit.UIPrinterPickerController, the method:

public virtual System.Threading.Tasks.Task<MonoTouch.UIKit.UIPrinterPickerCompletionResult>; PresentAsync (bool animated, out bool result);

contains an `out bool` parameter that captures (I think) whether or not the user selected a printer. The UIPrinterPickerCompletionResult that is passed to the Task<T> contains a field that does exactly this. There are other versions of PresentAsync that make more sense to asyncify.

This is not an isolated issue, as there are other places (in UIKit, alone) where this occurs.

Expected:
Maybe only asyncify the PresentAsync overload that does not contain an `out bool` parameter that is captured in the return value. (Though that sounded like snark, I meant only to convey that, while I can't see a reason for these bindings, there may be one of which I am not aware.)
Comment 1 Sebastien Pouliot 2017-08-15 21:32:06 UTC
Thanks @Mike!

There might be other cases but both `out` and `ref` are good signs of API that are not really usable with async.

What we need to do, in no particular order

1. btouch should emit a warning if `[Async]` is used on such API; it cannot be an error since it would break binary compatibility for anyone who made that mistake. It's possible to use `-warnaserror` to avoid future mistakes;

2. Our introspection tests should spot those conditions and report errors on the bots;

3. Obsolete our existing API that expose `out` or `ref` parameters and exclude them from XAMCORE_4_0;


@Alex please start with #1. I'll try to handle #2 and #3 independently.
Comment 2 Alex Soto [MSFT] 2017-11-28 23:13:20 UTC
PR: https://github.com/xamarin/xamarin-macios/pull/3059
Comment 3 Alex Soto [MSFT] 2017-11-28 23:29:21 UTC
Hello Mike I want to explain a bit more about the why this is not a bug in the current exposed API

This is the selector that 'UIPrinterPickerController.PresentAsync (bool animated, out bool result)' uses under the hood [1]

- (BOOL)presentAnimated:(BOOL)animated 
      completionHandler:(UIPrinterPickerCompletionHandler)completion;

The 'out bool result' is the actual boolean "Return Value" that the native selector returns, the documentations says that this value is

> YES if the picker was displayed or NO if the picker was already visible.

There is no other way to fetch this return value from our current [Async] implementation now answering your actual concern

> contains an `out bool` parameter that captures (I think) whether or not the user
> selected a printer. The UIPrinterPickerCompletionResult that is passed to the Task<T> 
> contains a field that does exactly this.

This is not completely true, as I mentioned above the 'out bool result' returns the selector result, true or false depending if picker was already visible according to docs [1], the actual bool you get from UIPrinterPickerCompletionResult[2] is the one that captures if the user selected a printer or if the user canceled the selection process.

So to recap a little if you use [Async] on a member that returns something we will generate a *Async method variant that takes an 'out var foo' so there is a way to fetch the actual return value of a selector and we'll also generate another variant of the same *Async method without the 'out var foo' so you can ignore the return value if you are not interested in it.

[1]: https://developer.apple.com/documentation/uikit/uiprinterpickercontroller/1620514-presentanimated?language=objc

[2]: https://developer.apple.com/documentation/uikit/uiprinterpickercompletionhandler?language=objc
Comment 4 Alex Soto [MSFT] 2017-11-28 23:30:58 UTC
Now after all the above explanation I must also thank you Mike for this bug report, we will now generate an error if someone tries to use [Async] with a method signature that takes an out or ref parameter.
Comment 5 Alex Soto [MSFT] 2017-11-28 23:33:15 UTC
Oh I should also mention something to @Sebastien: fortunately we do not have these mistakes on the API so there is no need for Obsolete'ing API or introspection tests, we generated uncompilable code so there is no way we are breaking someone :D