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

Api renewals #5

Merged
merged 9 commits into from
Dec 19, 2016
Merged

Api renewals #5

merged 9 commits into from
Dec 19, 2016

Conversation

acple
Copy link
Contributor

@acple acple commented Dec 15, 2016

the async method that returns ContextFreeTask means,

  • it is declaring "I will not hold the context".
  • it can 'Blocking-Wait' safely.
  • It is not be null.

i changed:

  • constructor makes to internal
  • if created by default keyword, it behaves same as FromResult(default(T))
  • add Wait method for blocking-wait
  • add EnableContextCapture method for context propagation
  • namespace restructuring

ContextFreeTaskはコンテキストをキャプチャするTaskを内部に持ってはならない。
よって、ContextFreeTaskはAsyncMethodBuilderからのみ作成されるべきである。
シグネチャがContextFreeTaskであるということは、すなわち「Waitしても安全」を意味する。
ContextFreeTaskをawaitした時点でContextが強制的に捨てられてしまうと、UIイベントハンドラ内などコンテキストが必要な場面でContextFreeTaskを返すAPIを一切使用することができなくなってしまう。
他の構造体がそうなってたので統一
自前定義が今後フレームワーク定義のものとぶつからないように
@@ -11,5 +11,6 @@ public struct ContextFreeTask
public Task Task => _task ?? FromResult(default(object));
internal ContextFreeTask(Task t) => _task = t;
public ContextFreeTaskAwaiter GetAwaiter() => new ContextFreeTaskAwaiter(Task);
public void Wait() => _task?.Wait();
Copy link
Owner

@ufcpp ufcpp Dec 16, 2016

Choose a reason for hiding this comment

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

This is by design. Users should use GetAwaiter().GetResult() instead of Wait(). The ValueTask doesn't have Wait too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in my opinion, naming of 'ContextFree' includes the meaning 'This can blocking-wait safely'.
Task and ValueTask should not use blocking-wait, but ContextFreeTask doesn't.
It is possible but, of course I think that is better not to use.

@@ -12,6 +12,6 @@ public struct ContextFreeTask
internal ContextFreeTask(Task t) => _task = t;
public ContextFreeTaskAwaiter GetAwaiter() => new ContextFreeTaskAwaiter(Task);
public void Wait() => _task?.Wait();
public ConfiguredTaskAwaitable ConfigureAwait(bool continueOnCapturedContext) => Task.ConfigureAwait(continueOnCapturedContext);
public ConfiguredTaskAwaitable EnableContextCapture() => Task.ConfigureAwait(true);
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer ConfigureAwait(true) because of consistency with Task and ValueTask.

internal ContextFreeTask(Task<T> t) => _task = t;
public ContextFreeTaskAwaiter<T> GetAwaiter() => new ContextFreeTaskAwaiter<T>(Task);
public void Wait() => _task?.Wait();
Copy link
Owner

Choose a reason for hiding this comment

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

by design, no Wait

@@ -9,7 +9,9 @@ public struct ContextFreeTask<T>
{
private readonly Task<T> _task;
public Task<T> Task => _task ?? FromResult(default(T));
public T Result => GetAwaiter().GetResult();
Copy link
Owner

Choose a reason for hiding this comment

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

This is uneasy. For consistency with ValueTask, the Result should be implemented like this. However, I don't prefer blocking wait.
If breaking change is allowed, I really want the Task.Result to throw an exception instead of blocking wait when it has not completed yet.

internal ContextFreeTask(Task t) => Task = t;
private readonly Task _task;
public Task Task => _task ?? FromResult(default(object));
internal ContextFreeTask(Task t) => _task = t;
Copy link
Owner

@ufcpp ufcpp Dec 16, 2016

Choose a reason for hiding this comment

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

So far, await on null should throw an Exception. For the purpose of null-safety, the constructor should throw an exception if t is null and _task should not be null. Of course, default(ContextFreeTask) makes _task be null, but, it is misuse.

I hope the following proposals:

dotnet/roslyn#7171
dotnet/roslyn#15108

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I concerned about default(ContextFreeTask). if provides struct as public, we must consider the default.
for example, ValueTask created with default keyword, it is same as FromResult(default(T)).
the constructor of ContextFreeTask is hid to internal. so ONLY default(ContextFreeTask) makes _task be null. it's very rare case.

@acple acple changed the title Apiの一新 Api renewals Dec 16, 2016
@ufcpp
Copy link
Owner

ufcpp commented Dec 19, 2016

OK, I'll accept your proposals.

@ufcpp ufcpp merged commit 180bbc5 into ufcpp:master Dec 19, 2016
@ufcpp ufcpp mentioned this pull request Dec 21, 2016
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.

2 participants