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.

Sign up for GitHub

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

Jump to bottom

Can we delete ValueTask.CreateAsyncMethodBuilder? #22741

Closed
stephentoub opened this issue Jul 12, 2017 · 1 comment
Closed

Can we delete ValueTask.CreateAsyncMethodBuilder? #22741

stephentoub opened this issue Jul 12, 2017 · 1 comment
Assignees
@stephentoub
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading
Milestone
2.1.0

Comments

@stephentoub
Copy link
Member

https://github.com/dotnet/corefx/blob/5a6d8ca975b512eeeea7404c740afcf865128405/src/System.Threading.Tasks.Extensions/src/System/Threading/Tasks/ValueTask.cs#L156-L161

This CreateAsyncMethodBuilder function was used as part of the initial C# compiler design for supporting arbitrary async return types, but the final design switched away from needing it. Unfortunately we neglected to then remove the method prior to shipping the library as stable.

The method is hidden from IntelliSense, and there's no good reason for someone to call it.

Can we remove it?
@karelz
Copy link
Member

karelz commented Sep 5, 2017

Approved.

Checked API usage (nuget and API port): no usage at all.
If anyone ever hits it, they can just use the default.

@stephentoub stephentoub self-assigned this Jan 30, 2018
jonpryor referenced this issue in xamarin/xamarin-android-api-compatibility Oct 8, 2018
@luhenry @jonpryor
Update BCL assemblies for mono/mono:2018-06@70fe915d breakage (#19)
7ccb480 
Context: dotnet/android#1836
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/4175/API_20Compatibility_20Checks/

The mono/mono:2018-06@70fe915d bump introduced "breaking" changes to
`System.Numerics.Vectors.dll` and `mscorlib.dll`.

`System.Numerics.Vectors.dll` "lost" the `System.Numerics.Vector` and
`System.Numerics.Vector<T>` types, because those were moved to
`mscorlib.dll`, with type forwarders in `System.Numerics.Vectors.dll`.
Unfortunately, `mono-api-html` doesn't understand type forwarders,
and thus reported their movement as breakage.

Update `reference/System.Numerics.Vectors.xml` accordingly.

The `mscorlib.dll` changes are a bit more complicated: the
`System.Buffers.IRetainable` type was removed, as were members
associated with that interface:

	<h3>Removed Type <span class='breaking' data-is-breaking>System.Buffers.IRetainable</span></h3>

	<!-- start type MemoryHandle --> <div>
	<h3>Type Changed: System.Buffers.MemoryHandle</h3>
	<p>Removed constructor:</p>
	<pre>
	  <span class='removed removed-constructor breaking' data-is-breaking>public MemoryHandle (IRetainable, void*, System.Runtime.InteropServices.GCHandle);</span>
	</pre>
	<p>Removed property:</p>
	<pre>
	  <span class='removed removed-property breaking' data-is-breaking>public bool HasPointer { get; }</span>
	</pre>

The `IRetainable` type was [accidentally shipped][0], and shouldn't
have been provided in the first place.

> [IRetainable] was design change before RTW unfortunately at that
> point we were not using stable branch of CoreFX so we accidentally
> shipped that API because we missed it when hiding the unstable
> Span APIs

Additionally, `System.Threading.Tasks.ValueTask<T>.CreateAsyncMethodBuilder()`
was removed:

	<h3>Type Changed: System.Threading.Tasks.ValueTask`1</h3>
	<p>Removed method:</p>
	<pre>
	  <span class='removed removed-method breaking' data-is-breaking>public static System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder&lt;TResult&gt; CreateAsyncMethodBuilder ();</span>
	</pre>

`ValueTask<T>.CreateAsyncMethodBuilder()` was removed because
[no code was found to be using it][1], and thus is deemed to be an 
acceptable ABI break.

Update `reference/mscorlib.xml` accordingly, and also include all the
new APIs that have been added since c550d1b.

[0]: dotnet/android#1836 (comment)
[1]: https://github.com/dotnet/corefx/issues/22171#issuecomment-327254875
jonpryor referenced this issue in xamarin/xamarin-android-api-compatibility Nov 29, 2018
@jonpryor
Reference API changes for mono/mono:2018-04@000780ca
5a5a2db 
Context: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android-d15-9/38/API_20Compatibility_20Checks/

The [xamarin-android commit to bump to mono/mono@000780ca][0]
introduced an [ABI break in `mscorlib.dll`][1]:

	<h3>Type Changed: System.Threading.Tasks.ValueTask`1</h3>
	<p>Removed method:</p>
	<pre>
	  <span class='removed removed-method breaking' data-is-breaking>public static System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder&lt;TResult&gt; CreateAsyncMethodBuilder ();</span>
	</pre>

`ValueTask<T>.CreateAsyncMethodBuilder()` was removed because
[no code was found to be using it][1], and thus is deemed to be an
acceptable ABI break.

Update `reference/mscorlib.xml` accordingly.

[0]: dotnet/android@0ea0725
[1]: https://github.com/dotnet/corefx/issues/22171#issuecomment-327254875
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@stephentoub stephentoub

Labels
api-approved API was approved in API review, it can be implemented area-System.Threading
Projects
None yet
Milestone
2.1.0
Development

No branches or pull requests
3 participants
@stephentoub @karelz @msftgits