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

Proposal: Asynchronous Construction #6788

Closed
alrz opened this issue Nov 15, 2015 · 341 comments
Closed

Proposal: Asynchronous Construction #6788

alrz opened this issue Nov 15, 2015 · 341 comments

Comments

@alrz
Copy link
Contributor

alrz commented Nov 15, 2015

Inspired by an article on async initialization:

It would be useful to be able to use await in a constructor, but this would mean that the constructor would have to return a Task<T> representing a value that will be constructed in the future, instead of a constructed value.

The point is, to be able to write:

class Foo {
    public async Foo() {
        // await
    }
}

To not alter the existing constructor semantics, the compiler would generate an async method corresponding to the async constructor and use it to initialize the object.

For local variable declaration we use

var foo = await new Foo();

This should be integrated with "async disposables" #114 and #5881 so that when it's out of scope, the disposal should be taken care of.

using foo = await new Foo();

It's a compiler error if one called an async constructor without await. in order to get the involving Task you may use the following syntax:

Foo foo = new Foo(); // compiler error
Task<Foo> foo = new async Foo(); // OK
Foo foo = await new Foo(); // OK

For initializing class fields we do as usual:

async class Bar {
    private Foo foo = await new Foo();
}

This makes the whole class async because you cannot instantiate this class with any of regular (or, obviously, async) constructors without await new or new async anymore.

Also, for async disposable types, the #111 should be taken into account.

async class Bar {
    private using Foo foo = await new Foo();
}

async class can be used also for classes that use await on methods in field initialization:

async class Baz {
    private Foo foo = await GetFooAsync();   
}

Then an await new or new async shall be needed in order to instantiate the type.

@CyrusNajmabadi
Copy link
Member

👍
I would love this. I was recently going through the process of virally spreading async through some pieces of the IDE code. This was no problem except when I ran into places where a constructor was calling something that I was converting to async. When that happened, I had to go through the process of introducing a static async factory method, and rejiggering things.

This wasn't too bad, except for when I had interesting constructors whose behavior now had to be mimicked with the factory methods. i.e. when you had this/base delegating constructors.

Having the compiler take care of this for me would be fantastic. Literally wonderful. It would make the process of converting sync to async that much more pleasant, and that much more uniform throughout the language.

@CyrusNajmabadi
Copy link
Member

Note: instead of:

await using foo = new Foo();

I would think it would be more like: await new Foo().

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@CyrusNajmabadi I thought it would be confusing that new Foo() would return a Task<Foo>, otherwise I'll be happy to see that too.

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

Since this Task mustn't be ignored, as @stephentoub once said I'm thinking about something like this

Foo foo = new Foo(); // compiler error
Task<Foo> foo = new async Foo(); // OK
Foo foo = await new Foo(); // OK

@HaloFour
Copy link

This would require CLR changes as constructors are invoked through a single IL opcode that is also responsible for allocation. Also, what would make this a particular improvement over a static initializer method?

public class Foo {
    private Foo() { } // sync initialization here

    public static async Task<Foo> Create() {
        Foo foo = new Foo();
        // async initialization here
        return foo;
    }
}

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@HaloFour I was considering "regular constructors" but the original proposal wasn't clear about this. Updated.

what would make this a particular improvement over a static initializer method?

I think the fact that compiler would handle all this would be really helpful as @CyrusNajmabadi said, in case of inheritance and this/base calls.

@HaloFour
Copy link

@alrz I agree it would be helpful. But that's still a pretty big change to the CLR. This could be accomplished through a pattern of exposed static methods:

public class Foo {
    protected Foo() { ... }

    public static Task<Foo> Create() {
        return Foo.Init(new Foo());
    }

    protected static async Task<Foo> Init(Foo foo) {
        // async initialization here
        return foo;
    }
}

public class Bar : Foo {
    protected Bar() { ... }

    public static Task<Bar> Create() {
        return Bar.Init(new Bar());
    }

    protected static async Task<Bar> Init(Bar bar) {
        bar = await Foo.Init(bar);
        // async initialization here
        return bar;
    }
}

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@HaloFour

still a pretty big change to the CLR

I think not. The compiler would generate an async method and it's responsible to emit the code to call it, so no "pattern" is needed. You just need an async constructor, that's it. Actually, the fact that you currently can not do this, seems a little unexpected. There are constructors and async in the language, so why can't you get both? If you had to write patterns for this kind of problems, and manage all this code all over the codebase, async wouldn't be there. I'd say it's boilerplate code, and since currently there is a feature that could do that, then it should be able to.

@HaloFour
Copy link

@alrz Because constructors aren't and cannot be normal methods. They cannot have return types. They can't even return the object being created. They cannot (well, in verified code) be executed directly except through the newobj IL opcode. And the newobj opcode cannot be applied to any method that isn't a constructor. The CLR has pretty strict rules regarding this so actually affecting how constructors work would definitely involve CLR changes.

The only way to avoid that would be to have the compiler translate constructor-like syntax into something like the static async method pattern that I described, but I think it would be very confusing to have new anything automatically translated into something that isn't a constructor call.

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@HaloFour await new and new async aren't clear enough? I suggested that a regular new should not be allowed with an async constructor.

@HaloFour
Copy link

@alrz

await can always be omitted when invoking an async method, it would be even more confusing to change that rule for this special case especially when you're not invoking a constructor anyway.

@bartdesmet
Copy link

Interesting thoughts. I think it could make sense to provide language syntax for this pattern (async modifier on ctor and await new would likely feel most natural) but there are a few hurdles I can think of. I'm on a mobile device now, so will be concise:

  • readonly fields can't be marked as initonly if the generated async method has to assign to them; how to keep guarantees when field is not private?
  • await new F() doesn't have a good place to put e.g. ConfigureAwait(false) - precedence and associativity of operations has to be considered but with compat in mind.
  • cross-language consumption could go via the emitted pattern if it looks any good, VB and C# could be cross-enlightened by means of a an attribute a la AsyncConstructorAttribute on a member or AsyncConstructorNameAttribute on the class (a la IndexerNameAttribute).
  • any VB-specific concerns if we'd want it there too (or at least the ability to consume an async ctor there).

More generally, async support is missing in quite a few places besides construction of objects, e.g. properties, indexers, foreach, using.

Pushing down async support to the CLR to help with some of these issues can't be ruled out but is rather unlikely I'd guess.

@bartdesmet
Copy link

One more thing wrt bullet two, note that await new isn't backwards compatible either; e.g. await new Task(...) would work today and mean something else than "async construction". Going down the slippery slope of debating syntax prior to a deep semantic debate, "async new" feels wonky because async has been used for declaration sites, and "awaitnew" to be different from "await new" isn't very satisfactory either. There's still "new await" of course :-).

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@bartdesmet

readonly fields can't be marked as initonly if the generated async method has to assign to them; how to keep guarantees when field is not private?

readonly fields are just a matter of metadata and compiler verification. even reflection can be used to modify a readonly field, so that wouldn't be a problem.

await new F() doesn't have a good place to put e.g. ConfigureAwait(false) - precedence and associativity of operations has to be considered but with compat in mind.

How about await new async F().ConfigureAwait(false);

More generally, async support is missing in quite a few places besides construction of objects, e.g. properties, indexers, foreach, using.

Nearly all of them are already proposed. foreach will be supported with async sequences I presume.

await new Task(...) would work today

The "async construction" only happens when the constructor is marked with async so that wouldn't be a problem either.

"async new" feels wonky because async has been used for declaration sites

I have suggested new async Foo() since the declaration for this constructor would be async Foo() I think that's just a good idea.

@HaloFour
Copy link

@alrz

readonly fields are just a matter of metadata and compiler verification.

And the CLR through verification and code access security.

@alrz
Copy link
Contributor Author

alrz commented Nov 15, 2015

@HaloFour If reflection can pass through this, I'm sure compiler would too.

@HaloFour
Copy link

@alrz The compiler cannot bypass the CLR checks. Direct IL manipulation of the field would be unverifiable. Emitting reflection code to manipulate the field would perform like garbage and that code would be subject to additional security constraints due to code access security.

@CyrusNajmabadi
Copy link
Member

Also, what would make this a particular improvement over a static initializer method?

As mentioned, it's a much cleaner form. It also makes it much easier to port over existing code to async. It makes the feature much less intrusive with existing language features. I can obviously get the same thing done with a static initializer. But it means a lot more munging around with code. It also means things like base-constructor delegation is far more annoying. I'd like the compiler to just do this for me.

As mentioned, I actually just had to deal with this while refactoring the IDE code. Constructors were, by far, the most annoying part of this change.

This is also an annoying issue when it comes to an automated refactoring. If the language has async constructors, then it becomes that much easier to provide a "convert this function to async (virally)". Without this feature, you have to do a lot more complicated conversions to make things work.

@HaloFour
Copy link

@CyrusNajmabadi

I doubt don't for a second that this was a tedious and annoying refactoring, but I don't think that it justifies such a drastic change to the nature of constructors (especially given it couldn't possibly be a constructor.) I'd say that your use case is a better argument for virtual static methods, although I think both are kludgy compared to a proper asynchronous factory pattern. I'd absolutely love a detailed playback as to how this proposal plays out during a language design meeting, though.

@KodrAus
Copy link

KodrAus commented Nov 16, 2015

Personally, I think that if you need to asynchronously create an object then you'd be better off with a factory. Is it useful practice to introduce io concerns to an object's constructor in a general enough sense to warrant language support?

@CyrusNajmabadi
Copy link
Member

but I don't think that it justifies such a drastic change to the nature of constructors (especially given it couldn't possibly be a constructor.)

Async methods are fairly drastically different under the covers. But the value is there :)

Personally, I think that if you need to asynchronously create an object then you'd be better off with a factory. Is it useful practice to introduce io concerns to an object's constructor in a general enough sense to warrant language support?

The "IO" concerns were there before. They were just blocking object construction. I don't se see how that's any better (or how it's any worse to be able to say that the object construction should be async).

I also don't see this as necessarily anything that requires CLR changes. This can simply be some sort of transformation that happens at the compiler level. Just as things like async/await, 'yield', and 'dynamic', end up causing massive transformations, but don't end needing any support at the CLR level.

@CyrusNajmabadi
Copy link
Member

Regardless, a real proposal would be necessary. but i'm totally behind the need for this. It's always annoying to me when you have language features that are great, until they intersect something else and fall over. "await + try/finally" was an annoying example of this. As is the inability to use things like async/await+yield together. In general, i'd prefer if you didn't continually run into these little annoying restrictions that make the features go from "omg great!" to simply "omg nice!" :)

@HaloFour
Copy link

@CyrusNajmabadi

Async methods are fairly drastically different under the covers.

This can simply be some sort of transformation that happens at the compiler level. Just as things like async/await, 'yield', and 'dynamic', end up causing massive transformations, but don't end needing any support at the CLR level.

Iterators and async methods are still normal CLR methods to the outside observer and their contract and metadata is exactly as they are written in the code. They're called as expected and behave as expected. For all of their internal machinations everything they do can be accomplished without the compiler candy and the consumer would be none-the-wiser. This particular syntax candy requires taking something described as a constructor and to change it into something that is not a constructor and cannot be used as a constructor. If a proposal moved away from that point I'd have no objections.

@alrz
Copy link
Contributor Author

alrz commented Nov 16, 2015

@HaloFour I wonder what you have in mind that a regular new on an async constructor should do, without changing the existing behavior.

@KodrAus
Copy link

KodrAus commented Nov 16, 2015

The "IO" concerns were there before. They were just blocking object construction. I don't se see how that's any better (or how it's any worse to be able to say that the object construction should be async).

My point is whether or not they should have been in the constructor in the first place. I see constructors that do any "work" as an anti-pattern, and so I see the idea of an async constructor as a feature that enables something you shouldn't do in the first place. But maybe my use-cases just aren't the ones that would benefit from this?

@CyrusNajmabadi
Copy link
Member

Iterators and async methods are still normal CLR methods to the outside observer and their contract and metadata is exactly as they are written in the code.

This is not always true though. 'Dynamic', for example, has a different contract than what's written in code as far the CLR is concerned. For example, a method that has 'dynamic' written as the return type actually has 'object' as its return as far as the CLR is concerned. But it still has vastly different semantics and is treated very differently when this is encountered by the C# compiler (or any other compiler that understands the attributes that C# emits here).

@CyrusNajmabadi
Copy link
Member

Similarly, operators and extension methods are special C# entities that use normal CLR methods, but allow use them in a special manner from C#. An operator is something special in C# that becomes just a normal CLR method. So why can't an 'async constructor' be something special in C# that becomes just a normal CLR method?

In both cases you have special syntax to declare them in the language, and there are then rules for how you use them. For example, when you make an operator (which just compiles down to a normal CLR method), you still can't call that method directly from C#. Instead, the C# compiler requires that you use the built in syntax (i.e. + instead of op_Addition) to do so.

So why can't this be the case for an async constructor? You have special syntax. It compiles to something special. And you have to consume it in a special way.

@HaloFour
Copy link

@CyrusNajmabadi

None of those redefine the nature or metadata of an existing CLR construct. They're just methods, perhaps decorated with attributes or just following a specific naming convention.

I also tend to agree with @KodrAus that a constructor is the wrong place to do this kind of work. Asynchronous or synchronous, if a type requires heavy lifting to construct that belongs in a factory method, not a constructor.

@alrz

I think that new should invoke constructors, and constructors can't be asynchronous.

@CyrusNajmabadi
Copy link
Member

None of those redefine the nature or metadata of an existing CLR construct

I never said async constructors would either.... so i don't see how that is relevant.

They're just methods

They're a C# construct that compiles down to a CLR method. Which, btw, is what i'm saying might be how we do this for async constructors.

perhaps decorated with attributes or just following a specific naming convention.

Which is also what i'm saying might be the way we do async constructors. It might have an attribute on it as well as a specific namign convention. It would need this (if we use methods) so that the compiler can recognize these when referencing an assembly so that you can use an "async constructor" from another compilation.

I also tend to agree with @KodrAus that a constructor is the wrong place to do this kind of work. Asynchronous or synchronous, if a type requires heavy lifting to construct that belongs in a factory method, not a constructor.

Why?

And, if that were the case, why do we allow any "heavy lifting" in the first place in a constructor?

I think that new should invoke constructors, and constructors can't be asynchronous.

I think . should only access fields, and that properties, namespaces and and methods need something else to access into. But we accept that it is valuable to provide a fair amount of consistency, uniformity and composability in the language.

To me, a factory method is necessary when you may not actually be returning an instance of your return type. i.e.:

static BaseType Create(parameters...)
{
    ... return new Derived1();
    ... return new Derived2();
}

I'm encapsulating that facility to make new things. However, when i know precisely what i want to make, and that thing knows how to make instances of itself, then i don't need a factory.

i don't see what "heavy lifting has to do with it either". Why on earth wouldn't i put whatever code made sense in a constructor? Say it does heavy lifting. Ok... so now i make it a factory... How have i helped the situation at all? My code still does heavy lifting... it just does it in a method, and then makes the object, instead of making it in the object itself. How is that code actually better? It's actually more confusing imo because now i have two ways to make the object. I can either use the factory or the constructor. And, even if i make the constructor private, i need to likely know that i shouldn't actually ever use it from within its body (except in the factory method).

You're now splitting two things that could be one, and it doesn't seem to buy much.

This is why i do not like that my async refactoring forces me into making factory methods. Instead of clear and concise code to begin with, i'm forced to introduce new concepts that i didn't have before. The goal of async/await was to prevent that. It was so that i could have sync code that i could easily make async by sprinkling the right keywords virally outward. imagine if we said "yup! that's how async/await works... except for while loops" (or, in reality "except for try/finally").

But, instead, we attempted to make it so that this feature worked well across a fairly large gamut of the language. As it turns out, the amount we got is really darn good. I've found it sufficient for a huge swath of the async work we do. However, there are still pitfalls, and i'd like to remove them to make it far easier and more composable to use this feature.

@Opiumtm
Copy link

Opiumtm commented Oct 24, 2016

@CyrusNajmabadi
I feel like there is nothing else to discuss.
There was exactly 4 major disagreements on this feature (as I followed the discussion)

  1. How it would map on CLR and reflection?
  2. It's a feature that would be prone to be abused
  3. It violate written OOP guidelines
  4. It don't add enough value to the language to worth

As discussion follows:

  1. Potentially maps well (I revoked this concern myself)
  2. Same abuse-prone level as async at whole. It's dangerous thing, but async already exists. So, async constructors don't add much abusive power over existing.
  3. You have convinced me that async isn't always used for resource-hungry operations, but also to abstract some work (which not always resource-intensive nor slow) with Roslyn itself as the good practical example. So, it doesn't violate established OOP guidelines much as basic rule remains same - don't do resource-intensive work in constructors, no matter sync or async.
  4. As 1, 2 and 3 points finally made clear it isn't much of issue.

So finally I vote "yes" for this feature and dismiss myself from discussion.
Taken alone this language feature don't deserve such a discussion (about good or bad this feature is) as conceptually it's extremely simple and conceptually there is nothing to much talk about. But it wasn't so obvious about risks.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 24, 2016

I feel like there is nothing else to discuss.

We have to decide on the semantics of the feature. :) How basic things like constructor chaining would actually work. Would there be issues inside structs with using async before all fields had been definitely assigned. How does one decide they want something like ValueTask? Can a synchronous constructor chain into an async constructor? etc. etc.

Again, the basic feature is incomplete. Hence why the later parts of the discussion are premature (especially as they assumed decisions being made on the CLR that we have not made).

@iam3yal
Copy link

iam3yal commented Oct 24, 2016

@CyrusNajmabadi

How does one decide they want something like ValueTask?

I don't know if it make sense but because all the operations are async once there's an async constructor, maybe, just maybe it make sense to dictate it based on the kind of type that you use? like if I use a class then it would be Task<> whereas if I use struct it would be ValueTask<>?

That's just an immediate gut feeling.

@Opiumtm
Copy link

Opiumtm commented Oct 24, 2016

@CyrusNajmabadi

How basic things like constructor chaining would actually work
How does one decide they want something like ValueTask?

First: Ideally - same as sync constructors work.
Async invokes on language level not much different from regular invokes as it doesn't change order of execution. It just yield control to other external code on same thread (not relevant to execution order and logic inside async method). It's just a form of "cooperative multitasking" preserving order of execution inside particular task.

Just after having to play with another proof-of-concept code (I wouldn't post it here as its implementation on current C# don't illustrate anything beyond proposed syntax)

public class Async1
{    
    // don't propose special syntax other than async keyword
    // but REQUIRE first parameter to be "out awaitable" with any name
    public async Async1(out ValueTask token, int someVal)
    {
        // some SYNC code goes here
        await something;
        // some async code goes here
        // no need to assign out awaitable parameter explicitly, it would be done by compiler
    }
}

public class Async2 : Async1
{
    public async Async2(out ValueTask token, int someVal, string val2)
        : base(out token, someVal)
    {
        // some SYNC code goes here
        await token; // it SHOULD be first await in chained async constructor
        // some async code goes here
    }
}

// Usage

var instance = await new Async1(10); // first "out awaitable" argument is omitted
var instance2 = await new Async2(15, "test");

var instance3 = await new Async1(out ValueTask *, 20); 
// first argument type is specified explicitly 
// using upcoming "wildcard" out argument syntax to just indicate argument type
// and don't care about its value

So, it would be this way:

  1. All sync code would be chained with same rules as with regular constructors, and would be executed immediately.
  2. Sync code in async constructor is considered any code before first await in constructor body
  3. All sync code in constructors chain would be executed before any async code in the chain. So order of execution would be: base class sync code -> this class sync code -> base class async code -> this class async code
  4. Chained constructor should await on base constructor's "out awaitable" token first. If it awaits on other awaitable before base token is awaited it should be considered compiler error. If none "await" in constructor body is provided, base constructor's awaitable token just passed through unmodified and chained constructor's code is considered entirely sync (any asynchrony in this constructor is inherited from the base constructor). Basically, async keyword may be omitted if chained constructor is sync - the only requirement for it is first out awaitable parameter (to pass through inherited awaitable token)
  5. Type of awaitable should be specified explicitly in out awaitable constructor parameter. This parameter should be always first. Allowed awaitable types here should be same as supported by async methods. So, it would be Task or ValueTask (any other awaitables are planned to be supported?)
  6. await new operator require first constructor parameter to be out awaitable (of any awaitable type: IAsyncOperation<T> or generic Task<T> could pass), this is the only requirement. So if constructor is made without async language feature, just declared out Task as first parameter (it's possible in C# already 😄 ) it would be OK to use await new on this constructor.
  7. If await new is used with constructor that has generic first out awaitable parameter (ValueTask<T> instead of ValueTask, for example) - it would be compiled with warning as task return value is ignored by await new operator. This is not an error, but it should throw warning.
  8. First "out awaitable" constructor's argument may be omitted in await new operator invoke. If there are 2 constructor oveloads with different awaitables (Task or ValueTask) as first argument, first argument should be specified explicitly. No "default behavior" should be chosen here.

@Opiumtm
Copy link

Opiumtm commented Oct 24, 2016

Some reasons why out awaitable syntax is proposed.

  1. It would be consistent with existing async design as async keyword don't modify member signature Same signature may be without async and it would be valid and provide essentially same API for the consumer. async keyword only modify internal details of member implementation by compiler and don't change anything in member signature.
  2. It would be consistent with constructor overloading. You explicitly declare first argument as out awaitable and it wouldn't mess with other constructor overloads and don't confuse anyone (no hidden arguments would be present).
  3. It impose minimal impact on existing syntax.
  4. It allow developer to declare actual awaitable type (Task or ValueTask)
  5. It allow developer to declare awaitable constructor don't using async keyword for constructor at all. Constructor can just return some arbitrary task in out argument and it would allow consumer to use await new operator.
  6. It would be consistent with CLR (yeah... premature...) as constructors allow to declare out arguments and it would be perfectly natural for existing CLR implementation to be implemented this way without the need to modify CLR and it would be transparent for reflection - reflected constructor will have same signature with out awaitable as first argument. All async constructor implementation details will be private, none of implementation details would leak into public API surface.

@HaloFour
Copy link

@eyalsk

So just because some of us got different ideas and opinions it makes us clueless about it?

Having the opinion is one thing. Actively claiming that the results of that opinion are in direct opposition to decades of research and aggregate development experience makes one "clueless" (your words, not mine). I can understand (and disagree) with the position that the language shouldn't absolutely force you onto the path of those documented best practices, but the statement that adhering to single-responsibility-principle and separation-of-concerns make for more difficult to test/maintain code is just ludicrous.

It's the same argument with liberals and conservatives, only it doesn't have to be cut and dry or about being wrong or right but merely different point of view.

Indeed, this conversation is "politically" charged and I've admitted several times that we're certainly not going to convince each other otherwise. It's not worth continuing to follow that line of reasoning. At this point it's like we're arguing climate change; sure, there are some different (and damned vocal) opinions on the subject, despite the vast prevailing consensus. 😉

I agree but in your view there's a contradiction and in our view there isn't.

A contradiction with what? My claim is that C# is as OOP as the majority of other OOP languages where the same best practice is documented. The claim that C# is somehow sufficiently different thus not under the scope of those guidelines is a contradiction.

I guess that here, properties should be just that return an already computed value.

Which is only a guideline, right? Property accessors are just methods, even moreso than constructors (the latter having much more rigid rules per the CLR). Why shouldn't the language enable and encourage developers to put all kinds of logic in them?

A million years ago the way you'd print a report in Crystal Reports was through writing to a property:

CrystalReport1.Action = crRunReport ' 1

The accessor would them block until the report was printed. If it's good enough for Crystal Reports surely it's good enough for C#? 😉

Yeah but maybe it's worth it for the sake of async and sync version feature parity?

But what if asynchronous constructors can't attain parity with synchronous constructors? Then you'd have something that looks and smells like a constructor, but can't be a constructor. That whole goal of being able to seamlessly refactor a synchronous constructor into an asynchronous constructor becomes impossible without further changes to the class. Is it really worth it then?

Again, this is why I bring up implementation. Ignoring whether or not this feature is a good idea from a design point of view, I think that there are still concerns which make it a bad idea from a functionality point of view. Resolving those issues requires either explicitly deciding to ignore them (a valid choice) or increasing the scope of work. All of a sudden that simple low-hanging fruit moves a few branches up that tree. Is the tree still worth climbing? Even if the answer is "yes" these are concerns that need to be addressed. I'm not satisfied with the answer that all of that can be deferred to some nebulous time in the future. We have the bandwidth and brainpower to at least go through some rough drafts and come up with action items.

Haha... maybe it can give you enough rope to pull you out of the pit? 😆

It's funny that you mention "pits". Do you think that asynchronous constructors represent a pit of success? Personally I don't, both from a design guideline and feature parity point of view. But we all know what they say about opinions: mine are sweet and fragrant. 😀

@iam3yal
Copy link

iam3yal commented Oct 24, 2016

@HaloFour Yup like you said we will never convince one another. 😉

A contradiction with what? My claim is that C# is as OOP as the majority of other OOP languages where the same best practice is documented. The claim that C# is somehow sufficiently different thus not under the scope of those guidelines is a contradiction.

Yeah, I meant that in our view we think that because async construction require "some work" before the object is initialized it isn't considered "the kind of work" they write about in guidelines whereas in your view it's a slippery slope, reasonably so I'd say but I still disagree. 😆

Which is only a guideline, right? Property accessors are just methods, even moreso than constructors (the latter having much more rigid rules per the CLR). Why shouldn't the language enable and encourage developers to put all kinds of logic in them?

You're right, I don't have a good argument against it but I can just say that conceptually they describe two different things, hence, they are expressed in the language as two different concepts.

The accessor would them block until the report was printed. If it's good enough for Crystal Reports surely it's good enough for C#?

Things change and maybe they are about to change again... 😄

But what if asynchronous constructors can't attain parity with synchronous constructors?

Once we will get into actual implementation details and there will be some blockers that will prevent it from doing it right I'd accept the judgment!

Do you think that asynchronous constructors represent a pit of success?

Still late and too far to tell, I have my doubts but I'm willing to have a discussion about it and see where we can go with it than doing nothing and wonder about it! that's where I stand, haha..

@joelharkes
Copy link

As I read all this (which is very interesting) I come to 2 conclusions:

*If constructor become async, then properties should definitely be async.

  • Wether or not you provide async constructors is a statement of where you want to go with the language: do you provide a language that helps you not to use anti patterns or do you make a language of ultimate freedom.

I think the person who "owns" c# should take this call or should decide on taking a developers survey to see if this feature is actually wanted a lot.

(I think more than half of c# developers are barely even using async yet)

@iam3yal
Copy link

iam3yal commented Nov 10, 2016

@joelharkes

(I think more than half of c# developers are barely even using async yet)

I think that this is a bad assumption. :)

@joelharkes
Copy link

I think not doing any heavy lifting (let's say it's out of process work, since this has async benefit) in the constructor is a good practice because it creates a dependency on this work and on the result.

Clearly if a class needs to prepare a lot of heavy lifting in the he constructor this class has 2 responsibilities now: 'prepare himself to be able to do his job' and 'do his job'. I'd say rather inject the result of the preparation into the class his constructor.

Another solution would be to use first time execution (heavy lifting on first execution, also called lazy execution). like an initialize function, but it is only executed when you need it, and cache it after first execution.

With all the developments on dependency injection. I think this problem can be fixed my making async resolve functionality in the di container, rather than implementing it in the language itself.

container.register(async (c) => {
  var lifter = c.resolve<Lifter>();
  var result = await lifter.DoSomeHeavyLifting();
  return new ClassInNeed(result);
}

Also doing heavy lifting inside a constructor makes unit testing slow or at least harder mockable.

Also a third party developer will think your constructor code is fast (same as him thinking properties are fast). Giving it extra capabilities for async constructors will only make it harder for 3rd party developers to learn & use your code properly.

I think time and energy is better spend in other language features.

@iam3yal
Copy link

iam3yal commented Nov 11, 2016

@joelharkes

Clearly if a class needs to prepare a lot of heavy lifting in the he constructor this class has 2 responsibilities now: 'prepare himself to be able to do his job' and 'do his job'. I'd say rather inject the result of the preparation into the class his constructor.

In your camp it does two things in our camp it does only one thing because my camp we don't think that the constructor is doing any work, it's all part of the object initialization.

Another solution would be to use first time execution (heavy lifting on first execution, also called lazy execution). like an initialize function, but it is only executed when you need it, and cache it after first execution.

This has nothing to do with this topic.

With all the developments on dependency injection. I think this problem can be fixed my making async resolve functionality in the di container, rather than implementing it in the language itself.

Not everyone serve their dependencies through a container.

Also doing heavy lifting inside a constructor makes unit testing slow or at least harder mockable.

This isn't true, the fact that you said that it will make testing slow just discredit you.

Also a third party developer will think your constructor code is fast (same as him thinking properties are fast). Giving it extra capabilities for async constructors will only make it harder for 3rd party developers to learn & use your code properly.

No, they wouldn't because they would have to call the constructor with async and it kinda implies that the operation won't finish immediately.

I think time and energy is better spend in other language features.

I think it's time to let the design team decide.

@joelharkes
Copy link

@eyalsk

This isn't true, the fact that you said that it will make testing slow just discredit you.

Ofcourse it makes your testing 'slow'. If you make your heavy lifting in your constructor. and you write n * amountOfMethods unit tests for this class. the constructor including heavy lifting is called n*amountOfMethods times. instead you should only inject the end result of the heavy lifting in the unit test so the tests stays fast + you only test the logic of the method and not the heavy lifting included. The heavy lifting should be in its own method which can be tested separately.

@iam3yal
Copy link

iam3yal commented Nov 11, 2016

@joelharkes I know that async is slower than synchronous calls due to all the magic behind that make this possible and I understand the point of separation of concerns or decoupling but I can't see why async by itself would be the source that drag your tests and make them slower.

Maybe I'm missing your point or misunderstand you, can you give me an example?

@joelharkes
Copy link

@eyalsk

No, your right, async constructors it self are not the bad, but rather what it provides. It would only provide a way to go against common practice and use your constructor for heavy lifting.

I do agree it's a nice feature but I also think it should only be used if you want to make something work quick and dirty.

This is why I stand by my view that other features should have more prio.

@iam3yal
Copy link

iam3yal commented Nov 11, 2016

@joelharkes

I do agree it's a nice feature but I also think it should only be used if you want to make something work quick and dirty.

I don't think that it applies only for when you want to get the job done and do things quick and dirty, people still use iterations over LINQ and interfaces over delegates when it make sense and it's right tool for the job.

I can certainly see the use of abuse here but I really don't think the language should educate you about usage, much like the English language by itself doesn't educate me to choose the words I use.

Yes I believe the language need to push developers into the pit of success if possible but not always, sometimes the developer need to be responsible to understand what he does.

@jnm2
Copy link
Contributor

jnm2 commented Nov 17, 2016

(Opiumtm) And same time they introduce additional risks - given an additional fact that async is already rather dangerous and shooting-in-your-own-leg prone feature.

Your perspective of the subject is certainly valid, but my confidence with async has only grown with experience. I think async/await is a stellar implementation on every design level and it never gets old using it where idiomatic.

When it comes right down to it for me, as a consumer of the language, I'm modeling concepts. And when the conceptual object that I'm modeling cannot possibly come into existence in any sort of valid state without waiting for an async call, it's ugly to not be able to express that via await new.

With the Task Initialized { get; } property, the abstraction is leaky. I still have to pick one of these options:

  • Check Initialized.IsCompleted and throw at the beginning of every property and method in the class
  • Convert all property getters and setters and methods into async methods which await Initialized
  • Make all constructors private and write a factory method per constructor as an arbitrary language workaround

This doesn't even get into the fact that I can't mark fields readonly that really should be.

All I want is an expressive model. No one reading my code will be confused and think that await new is doing less work than it actually is. There is no reasonable need to move that work somewhere else. There is a beautifully simple API for testing which is not impeded by this. The only need I have is to model an object which makes no sense to exist until its creation asynchronously completes. Please let me decide that I can handle the expressiveness and atomicity of await new.

@HaloFour
Copy link

@jnm2

This doesn't even get into the fact that I can't mark fields readonly that really should be.

"Asynchronous constructors" won't fix this problem. Any field you attempt to set after an await (even await base()) couldn't be readonly as the instance would already be created and that private constructor invoked. Sure, the CLR could be changed to allow this, but not without neutering initonly fields.

Please let me decide that I can handle the expressiveness and atomicity of await new.

The syntax can't be await new, that already means something in C# 5.0. Which means that "awaiting" an asynchronous constructor must look different than awaiting anything else.

@jnm2
Copy link
Contributor

jnm2 commented Nov 17, 2016

Sure, the CLR could be changed to allow this, but not without neutering initonly fields.

Initonly fields are already neutered once you move beyond the C# language into CLR reflection. My only expectation would be that it would just work and be enforced as a C# language feature. Nothing more. I don't actually care if they are initonly as long as the compiler prevents me from setting them outside the constructor. In fact, this seems like a fairly simple thing to implement.

The syntax can't be await new, that already means something in C# 5.0

This is new to me. Why? await new X(...), where X has a constructor with a matching signature marked async? I can't see that rule breaking existing code.

@HaloFour
Copy link

@jnm2

Initonly fields are already neutered once you move beyond the C# language into CLR reflection. My only expectation would be that it would just work and be enforced as a C# language feature. Nothing more.

Reflection requires full trust. Making readonly only a "suggestion" completely removes the facility of the feature. And any accessibility above private would be completely useless.

Why? await new X(...), where X has a constructor with a matching signature marked async?

The existing rule is that await applies to the result of the expression, which in this case is the constructed instance. That is perfectly legal code in C# 5.0 if X is awaitable, which it could be through extension methods. To avoid breaking changes the compiler would have to consider both cases and prefer the possibility of extension methods. That's assuming the compiler could handle the parsing ambiguity.

@jnm2
Copy link
Contributor

jnm2 commented Nov 17, 2016

@HaloFour That's a good argument for CLR async initonly support. I don't see how it's that different from existing behavior. There is a period of time where the fields can be assigned from constructor method bodies and the instance cannot be observed except inside the constructor, and then the construction is over and further assignments are not possible and the instance can be observed by the outside world.

That is perfectly legal code in C# 5.0 if X is awaitable, which it could be through extension methods.

Of course, but that is fringe to non-existent and shouldn't hold this proposal back. If instance methods or extension methods exist which make X an awaitable primitive, of course the semantics change. I have no problem with that.

@Thaina
Copy link

Thaina commented Jan 11, 2017

I have 2 mind for this

At first I think I would go against this idea. Because constructor should always be synchronous in memory. And if you need to load something it should be factory

But on the other hand. Factory is so hard to extend. Generic constraint on new cannot contain parameter. We cannot write interface for constructor. And et cetera; all of these problem lead to a difficulty to do anything with constructor indirectly while we can only set readonly in constructor

So to have sophisticate async factory we need to implement so manything. async constructor would be fastest way to solve this problem
Still I prefer async factory instead of constructor

@Pzixel
Copy link

Pzixel commented Jan 12, 2017

Here is example of how async constructor can be implemented with csc.exe-only conversion (i.e. independed from CLR)

public class A
{
	public readonly int X;
	public async A(int x)
	{
		await Task.Delay(0).ConfigureAwait(false);
		X = x;
	}
}

public class B : A
{
	public readonly int Y;
	public async B(int x, int y) : base(x)
	{
		await Task.Delay(10).ConfigureAwait(false);
		Y = y;
	}
}

which could be translated into:

public class A
{
	public readonly int X;
	protected A(int x)
	{
		X = x;
	}

        [AsyncConstructor]
	public static async Task<A> <A>__CreateAsync(int x)
	{
		await Task.Delay(0).ConfigureAwait(false);
		return new A(x);
	}
}

public class B : A
{
	public readonly int Y;
	protected B(int x, int y) : base(x)
	{
		Y = y;
	}

        [AsyncConstructor]
	public static async Task<B> <B>__CreateAsync(int x, int y)
	{
		await Task.Delay(0).ConfigureAwait(false);
		var <x>__Field = x; // instead of setting field we are setting local variable. We can further use it, there is no difference for the code below
		await Task.Delay(10).ConfigureAwait(false);
		var <y>__Field = y;
		return new B(<x>__Field, <y>__Field);
	}
}

I am using fields because properties may have some additional logic and probably cannot be replaced with local variables. More investigations are required here, but that looks good for proof-of-concept

@HaloFour
Copy link

@Pzixel

That method breaks down when A is defined in a different assembly or you need to use instance members of the current or base type. The number of caveats would make it fairly impractical.

@Thaina
Copy link

Thaina commented Jan 13, 2017

Now I see why we should not have async constructor

What happen if both A and B have many chain of await? A as parent would finish construction in one thread but then B still unfinished and wait for something to finished in another thread. We end up as a half finish object sometimes

This cannot translate into factory automatically. And how could runtime handle half finish object?

@Pzixel
Copy link

Pzixel commented Jan 16, 2017

@Thaina I can't imagine any visible to client state when you have an access to partialy-inited object. We are creating empty object of type B. Then we are calling constructor A on it like it was A object. When A constructor is finished we are executing an actual B body and return constructed object. It's not what I wrote above but what I consider now as viable solution. It's abolutely doable, but requires CLR support unlike C#-only way, which I currently find to be non existing.

@Thaina
Copy link

Thaina commented Jan 23, 2017

@Pzixel You are right if we have CLR support it directly. I mean we can't workaround with factory only by compiler

@MgSam
Copy link

MgSam commented Apr 6, 2017

@alrz Can you port this issue to the CSharpLang repo?

@alrz
Copy link
Contributor Author

alrz commented Apr 7, 2017

@MgSam Yes. I'm on it.

@alrz
Copy link
Contributor Author

alrz commented Apr 7, 2017

Moved to dotnet/csharplang#419

@alrz alrz closed this as completed Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests