dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.74k stars 3.99k forks source link

C# Design Notes for Sep 8, 2015 #5383

Closed MadsTorgersen closed 8 years ago

MadsTorgersen commented 8 years ago

C# Design Notes for Sep 8, 2015

Agenda

  1. Check-in on some of our desirable features to see if we have further thoughts
  2. Start brainstorming on async streams

    Check-in on features

    Bestest betterness

Overload resolution will sometimes pick a candidate method only to lead to an error later on. For instance, it may pick an instance method where only a static one is allowed, a generic method where the constraints aren't satisfied or an overload the relies on a method group conversion that will ultimately fail.

The reason for this behavior is usually to keep the rules simple, and to avoid accidentally picking another overload than what you meant. However, it also leads to quite a bit of frustration. It might be time to sharpen the rules.

Private protected

In C# 6 we considered adding a new accessibility level with the meaning protected and internal (protected internal means protected or internal), but gave up because it's hard to settle on a syntax. We wanted to use private protected but got a lot of loud push back on it.

No-one has come up with a better syntax in the meantime though. We are inclined to think that private protected just takes a little getting used to. We may want to try again.

Attributes

There are a number of different features related to attributes. We should return to these in a dedicated meeting looking at the whole set of proposals together:

A lot of these would be particularly helpful to Roslyn based tools such as analyzers and metaprogramming.

Local extern functions

We should allow local functions to be extern. You'd almost always want to wrap an extern method in a safe to call wrapper method.

Withers for arbitrary types

If we want to start focusing more on immutable data structures, it would be nice if there was a language supported way of creating new objects from existing ones, changing some subset of properties along the way:

var newIfStatement = ifStatement with { Condition = newCondition, Statement = newStatement }; // other properties copied 

Currently the Roslyn code base, for example, uses the pattern of "withers", which are methods that return a new object with one property changed. There needs to be a wither per property, which is quite bloatful, and in Roslyn made feasible by having them automatically generated. Even so, changing multiple properties is suboptimal:

var newIfStatement = ifStatement.WithCondition(newCondition).WithStatement(newStatement); 

It would be nice if we could come up with an efficient API pattern to support a built-in, efficient with expression.

Related to this, it would also be nice to support object initializers on immutable objects. Again, we would need to come up with an API pattern to support it; possibly the same that would support the with expression.

Params IEnumerable

This is a neat little feature that we ultimately rejected or didn't get to in C# 6. It lets you do away with the situation where you have to write two overloads of a method, one that takes an IEnumerable<T> (for generality) and another one that takes params T[] and calls the first one with its array.

The main problem raised against params IEnumerable is that it encourages an inefficient pattern for how parameters are captured: An array is allocated even when there are very few arguments (even zero!), and the implementation then accesses the elements through interface dispatches and further allocation (of an IEnumerator).

Probably this won't matter for most people - they can start out this way, and then build a more optimal pattern if they need to. But it might be worthwhile considering a more general language pattern were folks can build a params implementation targeting something other than arrays.

Async streams

We shied back from a notion of asynchronous sequences when we originally introduced async to C#. Part of that was to see whether there was sufficient demand to introduce framework and language level concepts, and get more experience to base their design on. But also, we had some fear that using async on a per-element level would hide the true "chunky" degree of asynchrony under layers of fine-grained asynchronous abstractions, at great cost to performance.

IAsyncEnumerable

At this point in time, though, we think that there is definitely demand for common abstractions and language support: foreach'ing, iterators, etc. Furthermore we think that the performance risks - allocation overhead in particular - of fine grained asynchrony can large be met with a combination of the compiler's existing optimizations and a straightforward asynchronous "translation" of IEnumerable<T> into IAsyncEnumerable<T>:

public interface IAsyncEnumerable<T>
{
  public IAsyncEnumerator<T> GetEnumerator();
}

public interface IAsyncEnumerator<T>
{
  public T Current { get; }
  public Task<bool> MoveNextAsync();
}

The only meaningful difference is that the MoveNext method of the enumerator interface has been made async: it returns Task<bool> rather than bool, so that you need to await it to find out whether there is a next element (which you can then acquire from the Current property) or the sequence is finished.

Allocations

Let's assume that you are foreach'ing over such an asynchronous sequence, which is buffered behind the scenes, so that 99.9% of the time an element is available locally and synchronously. Whenever a Task is awaited that is already completed, the compiler avoids the heavy machinery and just gets the value straight out of the task without pause. If all awaited Tasks in a given method call are already completed, then the method will never allocate a state machine, or a delegate to store as a continuation, since those are only constructed the first time they are needed.

Even when the async method reaches its return statement synchronously, without the awaits having ever paused, it needs to construct a Task to return. So normally this would still require one allocation. However, the helper API that the compiler uses for this will actually cache completed Tasks for certain common values, including true and false. In summary, a MoveNextAsync call on a sequence that is buffered would typically not allocate anything, and the calling method often wouldn't either.

The lesson is that fine-grained asynchrony is bad for performance if it is done in terms of Task<T> where completed Tasks are never or rarely cached, e.g. Task<string> or Task<int>. It should be done in terms of Task<bool> or even non-generic Task.

We think that there may or may not be scenarios where people want to get explicit about the "chunks" that data is transmitted in. If so, they can express this as IAsyncEnumerable<Chunk<T>> or some such thing. But there is no need to complicate asynchronous streaming by forcing people to deal with chunking by default.

Linq bloat

Another concern is that there are many API's on IEnumerable<T> today; not least the Linq query operators Select, Where and so on. Should all those be duplicated for IAsyncEnumerable<T>?

And when you think about it, we are not just talking about one extra set of overloads. Because once you have asynchronously foreach'able streams, you'll quickly want the delegates applied by the query operators to also be allowed to be async. So we have potentially four combinations:

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate);
public static IAsyncEnumerable<T> Where<T>(this IAsyncEnumerable<T> source, Func<T, bool> predicate);
public static IAsyncEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, Task<bool>> predicate);
public static IAsyncEnumerable<T> Where<T>(this IAsyncEnumerable<T> source, Func<T, Task<bool>> predicate);

So either we'd need to multiply the surface area of Linq by four, or we'd have to introduce some new implicit conversions to the language, e.g. from IEnumerable<T> to IAsyncEnumerable<T> and from Func<S, T> to Func<S, Task<T>>. Something to think about, but we think it is probably worth it to get Linq over asynchronous sequences one way or another.

Along with this, we'd need to consider whether to extend the query syntax in the language to also produce async lambdas when necessary. It may not be worth it - using the query operators may be good enough when you want to pass async lambdas.

Language support

In the language we would add support for foreach'ing over async sequences to consume them, and for async iterators to produce them. Additionally (we don't discuss that further here) we may want to introduce a notion of IAsyncDisposable, for weach we could add an async version of the using statement.

One concern about async versions of language features such as foreach (and using) is that they would generate await expressions that aren't there in source. Philosophically that may or may not be a problem: do you want to be able to see where all the awaiting happens in your async method? If that's important, we can maybe add the await or async keyword to these features somewhere:

foreach (string s in asyncStream) { ... } // or
foreach async (string s in asyncStream) { ... } // or
foreach (await string s in asyncStream) { ... } // etc.

Equally problematic is when doing things such as ConfigureAwait, which is important for performance reasons in libraries. If you don't have your hands on the Task, how can you ConfigureAwait it? The best answer is to add a ConfigureAwait extension method to IAsyncEnumerable<T> as well. It returns a wrapper sequence that will return a wrapper enumerator whose MoveNextAsync will return the result of calling ConfigureAwait on the task that the wrapped enumerator's MoveNextAsync method returns:

foreach (string s in asyncStream.ConfigureAwait(false)) { ... }

For this to work, it is important that async foreach is pattern based, just like the synchronous foreach is today, where it will happily call any GetEnumerator, MoveNext and Current members, regardless of whether objects implement the official "interfaces". The reason for this is that the result of Task.ConfigureAwait is not a Task.

A related issue is cancellation, and whether there should be a way to flow a CancellationToken to the MoveNextAsync method. It probably has a similar solution to ConfigureAwait.

Channels in Go

The Go language has a notion of channels, which are communication pipes between threads. They can be buffered, and you can put things in and take them out. If you put things in while the channel is full, you wait. If you take things out while it is empty, you wait.

If you imagine a Channel<T> abstraction in .NET, it would not have blocking waits on the endpoints; those would instead be asynchronous methods returning Tasks.

Go has an all-powerful language construct called select to consume the first available element from any set of channels, and choosing the logic to apply to that element based on which channel it came from. It is guaranteed to consume a value from only one of the channels.

It is worthwhile for us to look at Go channels, learn from them and consider to what extent a) we need a similar abstraction and b) it is connected to the notion of async streams.

Some preliminary thoughts: Channels and select statements are very easy to understand, conceptually. On the other hand they are somewhat low-level, and extremely imperative: there is a strong coupling from the consumer to the producer, and in practice there would typically only be one consumer. It seems like a synchronization construct like semaphores or some of the types from the DataFlow library.

The "select" functionality is interesting to ponder more generally. If you think about it from an async streams perspective, maybe the similar thing you would do would be to merge streams. That would need to be coupled with the ability to tie different functionality to elements from different original streams - or with different types. Maybe pattern matching is our friend here?

foreach (var e in myInts.Merge(myStrings))
{
  switch (e)
  {
    case int i:
      ...
    case string s:
      ...
  }
}

Either way, it isn't as elegant by a long shot. If we find it's important, we'd need to consider language support.

Another important difference between IAsyncEnumerable and Channels is that an enumerable can have more than one consumer. Each enumerator is independent of the others, and provides access to all the members - at least from the point in time where it is requested.

Conclusion

We want to keep thinking about async streams, and probably do some prototyping.

JoshVarty commented 8 years ago

Currently the Roslyn code base, for example, uses the pattern of "withers", which are methods that return a new object with one property changed. There needs to be a wither per property, which is quite bloatful, and in Roslyn made feasible by having them automatically generated.

"Withers for arbitrary types" sounds really cool. I'm not sure how it would work, though. Would this even feature be able to save Roslyn any code? My understanding is that under the hood the mutable green trees get mutated. I'm just not sure how you could do all this automatically, but maybe I'm misunderstanding the feature.

HaloFour commented 8 years ago

For private protected I think that if you're planning on implementing the feature just stuck with that original keyword combination and pull the trigger. By even putting it up for consideration you're just inviting the people who won't be happy with anything. C++/CLI already calls that access private protected so at least it would be consistent.

I'm a fan of "withers" being a new with operator limited to returning the same type as the declaring type and requiring a parameter for each of the public fields/properties. Record types could automatically generate the with operator so you wouldn't need a special implementation for record types v. other types.

I think that copying Go doesn't make a lot of sense. They needed select as otherwise receiving from a channel is a blocking operation. C#'s async/await is significantly superior when it comes to scalar operations, and I would argue that Rx is superior for stream/channel operations. The use cases selecting over multiple asynchronous operations are, in my opinion, too specialized to be served by one syntax. There are concerns with cancellation, exception handling, etc. that would need to be addressed. The same functionality could be accomplished through a fluent API without having to touch the language.

Hosch250 commented 8 years ago

One potential syntax for a "wither" could be similar to the object initialization syntax:

var cat = new Cat { Age = 10, Name = "Fluffy" };

See this webpage: https://msdn.microsoft.com/en-us/library/bb384062.aspx

So, perhaps we could do:

var cat = new Cat { Age = 10, Name = "Fluffy" };
cat = cat { Age = 1, Name = "FizzBuzz" };

instead of

var cat = new Cat { Age = 10, Name = "Fluffy" };
cat = cat.WithAge(1).WithName("FizzBuzz);
bombless commented 8 years ago

var cat = new Cat { Age = 10, Name = "Fluffy" }; cat { Age = 1, Name = "FizzBuzz" };

+1, hope this is doable.

JoshVarty commented 8 years ago

You guys mean

cat =cat { Age = 1, Name = "FizzBuzz" };

Right? The proposal is for immutable objects, so you'll need to capture that result somehow.

andrewtobin commented 8 years ago

I don't like the use of "private protected" as a name solely because you don't intend for it to be "private" but "internal" - is there some symbol that could be used as an operator? I'm only reading this out of interest and I don't know the ins and outs of language design so forgive me, but something like protected &internal - although I guess there's a memory addressing look to it, or protected +internal (the former makes more sense because you could do protected |internal to cover existing, and be explicit).

andrewtobin commented 8 years ago

With regards to Withers - could you see any value in having: var newIfStatement = ifStatement replaces { Condition = newCondition, Statement = newStatement };

I would assume it would throw if the properties didn't already exist, otherwise you'd need to:

Animal ifStatement;

Cat newIfStatement = ifStatement as Cat replaces { Condition newCondition, Statement = newStatement };

paulomorgado commented 8 years ago

No-one has come up with a better syntax in the meantime though. We are inclined to think that private protected just takes a little getting used to. We may want to try again.

Correction: I came up with a better syntax: protectedinternal :smile:

HaloFour commented 8 years ago

@Hosch250 The point of "withers" is specifically to "copy" the object to a new instance with specific changes. Using it to affect an existing instance doesn't make a lot of sense.

var cat1 = new Cat { Name = "Fluffy", Gender = Gender.Female, Age = 6 };
var cat2 = cat1 with { Name = "Whumpus" };

Debug.Assert(cat1.Gender == cat2.Gender);
Debug.Assert(cat1.Age == cat2.Age);

If a custom "wither" was implemented as an instance method then it would certainly be possible to affect the current instance. Although I am a proponent of going the operator route as being completely new syntax would give the compiler some additional control over how they are used, e.g. the compiler could fail to compile a class with a custom with operator if the static method does not adhere specifically to the expected signature.

@paulomorgado You and everyone else, which was my point. Can't please anyone.

eamodio commented 8 years ago

@paulomorgado or something like proternal :)

accidentaldeveloper commented 8 years ago

Regarding private protected, I'm still partial to the suggestion that @MadsTorgersen made when this feature was proposed in 2014: create a new keyword to represent this specific accessibility. My favorite choice is restricted but others are certainly acceptable. I find this much less cognitively dissonant than using private protected.

A brand new keyword has the distinct benefit that on first sight all naïve developers would be equally uncertain of its meaning. private protected offers no benefit in this aspect: the modifiers' current meanings are not compatible and do not lead to the combined meaning that they are planned to represent. A C# developer, naïve of C# 7, would be hopeless to make accurate conclusions about this composite construction. Only by reading the documentation would they discover the correct meaning. In contrast, with restricted it's immediately clear that this is something entirely new to C#.

Using a modifier like "and", "or", or "intersection" in the keyword (protectedIntersectionInternal) would be the most explicit but this style doesn't seem to fit with the rest of C#, or the bulk of C# developers. The protected internal modifier is already confusing and misplaced. I see no reason to double down with private protected.

Hosch250 commented 8 years ago

@HaloFour @JoshVarty Yes, I meant to re-assign, not mutate. I have fixed this.

daveaglick commented 8 years ago

+1 for improvements to overload resolution rules, especially if considering generic constraints is on the table. I know that has it's own set of challenges since constraints aren't currently part of the signature (#250 and #962), but if that could be overcome it would be a welcome refinement.

JasonBock commented 8 years ago

+1 on the attribute stuff. Having compile-time attributes that worked on the source code would start to get down some very cool metaprogramming avenues. (And generic attributes, please! :))

ggreig commented 8 years ago

+1 to private protected. I don't like the syntax, but would accept it for the functionality. It would be worth seeing if a brand new keyword could be arrived at - I don't suppose familysecret will fly? ;-)

MgSam commented 8 years ago

Correction: I came up with a better syntax: protectedinternal :smile:

Regarding private protected, I'm still partial to the suggestion that @MadsTorgersen made when this feature was proposed in 2014...

Oh God, please stop. Not this argument again. C# team- you guys decide what to call it. I don't care if you call it qwerty or asdfgh or hejlsberg or sweetfluffybunny. The bottom line is that the feature is useful and the community will never be happy no matter what keyword you choose, so just pick one.

I'm looking forward to seeing a more fleshed out design for the async streams. This will be a great area for people to play around with a prototype and compare it to using Rx, vanilla async.

omariom commented 8 years ago

if async foreach is pattern based why not make the result of IAsyncEnumerator.MoveNextAsync() pattern based too? Then it could be smth more efficient than Task from allocation point of view. Just anything having instance or extension GetAwaiter method.

omariom commented 8 years ago

private protected??
Novices will get confused..

Hosch250 commented 8 years ago

I dislike private protected too - it doesn't say what it really is. I like @accidentaldeveloper's suggestion of restricted.

HaloFour commented 8 years ago

Seriously, that horse was beaten beyond recognition. Giant threads, surveys, flames galore. The consensus was that there is no consensus. There are more opinions than participants. It's not a useful dialog.

For a feature that might be used a handful of times by a small minority of the development community the team should just pull the trigger (and go with their gut regarding the keyword(s)) or pull the plug.

chaosrealm commented 8 years ago

How about 'assembly protected' for the new visibility scope?

Hosch250 commented 8 years ago

@chaosrealm Then what would assembly stand for on its own?

omariom commented 8 years ago

@MadsTorgersen

The Go language has a notion of channels, which are communication pipes between threads.

Did you consider TPL Dataflows's IPropagatorBlock for this?

svick commented 8 years ago

A crazy idea: if you have several asynchronous operations, it often makes sense to let them execute concurrently. To make this easier, async foreach could be an expression that returns a Task representing completion of the loop. E.g.:

var task1 = async foreach (string s in asyncStream1)
{
    // loop body
};

var task2 = async foreach (int i in asyncStream2)
{
    // loop body
};

await Task.WhenAny(task1, task2);

One reason why this is crazy is that it would probably be hard to make the statement form and the expression form work well at the same time. (Are they distinct? Very confusing. Are they the same? That would mean you would have to write something like await async foreach (…) { … };, which looks really weird, and you would also have to be very careful about not forgetting that await.)

svick commented 8 years ago

Regarding Go's select, a very similar feature for async non-streams has been proposed at #5187.

accidentaldeveloper commented 8 years ago

@MgSam @Hosch250 I agree: the naming is contentious but the feature is useful: the C# team should make a decision and live with it. Some users will feel vindicated, others will feel slighted, and we'll all get on with our lives. The feature is definitely more important than the name at this point.

RichiCoder1 commented 8 years ago

Private protected

Agreeing with above commenters here. Pick a name and live with it.

Attributes

Yes, so much +1. Generic attributes would be great a for library authors, and compiler attributes will keep getting that much more powerful thanks to Roslyn. I could easy imagine pairing compiler attributes with "core-aware" libraries to provide extremely intelligent guidance.

Async streams

All around fully for this and agree with the teams thought process. Channels would certaintly be interesting, but as others have mentioned it seems somewhat redundant with Dataflow. Maybe take lessons from them and beef up language support for their contracts? Also, I mentioned this on the Async Streams thread but first class language support for push would be great, though it sounds like channels somewhat accomplish this.

ErikSchierboom commented 8 years ago

I love the proposed with syntax. It is concise and unambiguous.

TonyValenti commented 8 years ago

Hi All, As I was reading through this, it reminded me of a few ideas I've had floating around in my head.

Regarding Withers, currently it sounds like there would be a lot of generated code and it made me think of this issue #5292 related to source control tooling. For something like Withers, I feel like we should be able to apply attributes to either classes or even declaration statements to change them. In the case of Withers, we should be able to do something like this:

[Withers]
public class Cat {
  public property string Name {get; set;}
  public property int Age {get; set;}
}

....

var pet = new Cat();
var pet2 = pet.WithName("Cuddles").WithAge(6);

and have it automatically generate the WithName and WithAge methods. Additionally, I should be able to apply Withers to an existing class and have it generate extension methods that allow me to do the same thing:

//No Withers attribute on the class.
public class Cat {
  public property string Name {get; set;}
  public property int Age {get; set;}
}

....
//Withers attribute applied to the declaration.
[Withers]
var pet = new Cat();
var pet2 = pet.WithName("Cuddles").WithAge(6);

I had also been thinking about attributes on statements related to async operations. Right now, using the Parallel.ForEach methods and such requires a bit of clunky syntax. In the case that I have a loop that I want to be parallel, what if I could simply do this:

[Parallel]
foreach(int i = 0; i < 100; ++i){
    //Do Something
}

I will say, though, that I do really like @svick 's recommendation on syntax for returning the task. I'm assuming that in this situation, not only would it be Async, but it would also use the Parallel.ForEach under the hood, correct?

HaloFour commented 8 years ago

@TonyValenti

That approach for "withers" was mentioned in #5172 (Comment). Having to work with multiple intermediate allocations is probably enough reason to not go that route.

alrz commented 8 years ago

What about using the existing IObservable<> interface for async streams?

async IObservable<int> F() {
    foreach(var item in list)
        yield return await item.MethodAsync();
}

Then foreach would support the IObservable<> instead of IAsyncEnumerable<> — acting as an IObserver<>:

foreach(var item in observable) { }

Making it an expression (#5402) might be useful (in @svick's example):

var task1 = foreach (string s in asyncStream1) =>
{
    // loop body
};

var task2 = foreach (int i in asyncStream2) =>
{
    // loop body
};

await Task.WhenAny(task1, task2);

I assumed that the Task<> class have implemented the IObservable<> interface.

axel-habermaier commented 8 years ago

private protected: just do it! It is rarely used anyway but I've missed it on more than one occasion...

HaloFour commented 8 years ago

@alrz Check out the comments in #261 re async streams and IObservable<T>. Needless to say I'm a huge proponent and I think supporting both would be useful. The argument against language support for IObservable<T> streams being that Rx makes them so easy to create. As for consuming an IObservable<T> I imagine that it will be very simple to write an extension method that adapts to IAsyncEnumerable<T>.

@axel-habermaier Curious as to what scenarios you would've found private protected (or whatever) useful where internal would not suffice? Specifically to keep other members of your team/whatever from accidentally using it outside of derived classes within the assembly?

alexpolozov commented 8 years ago

@HaloFour private protected is currently the best way to replicate "closed records" — a functionality that can only be implemented by a fixed number of classes in a closed hierarchy.

alrz commented 8 years ago

@HaloFour My point is, the Task<> is an IObservable<> at heart, no matter for a single value or a sequence of values. And it's push-based. To quote @tpetricek:

You ask it to start, and then it keeps throwing values at you.

so does Task<>. And since IObservable<> exists in the BCL, why doesn't Task<> implement it?

As it turns out, while Task<TResult> does not currently implement IObservable<T>, it’s actually quite a good fit to do so. An observable represents any number of values terminated by either an end of the stream or by an exception. A Task<TResult> fits this description: it completes with either a single value or with an exception.

I think that this syntax for supporting yield return is consistent and makes sense:

IEnumerable<int> F() {  /* contains yield return */  }
async IObservable<int> G() {  /* contains yield return & await */  } 

In fact, the latter returns a Task<> which have implemented the IObservable<>.

HaloFour commented 8 years ago

@alrz I'd agree with you. A Task<T> is a simple IObservable<T> of a single value. Rx already contains an extension method to wrap a Task<T> as an IObservable<T>. However, an IObservable<T> is not a Task<T> as the former can represent more than one result, e.g.:

public async IObservable<int> G() {
    for (int i = 0; i < 10; i++) {
        await Task.Delay(1000);
        yield return i;
    }
}

The big difference between IAsyncEnumerable<T> and IObservable<T> is that the former completely pauses at yield return and doesn't resume until the consumer invokes MoveNext. The latter could theoretically continue immediately after yield return (as IObserver<T>.OnNext returns) depending on how it is being consumed, back pressure buffering, etc.

HaloFour commented 8 years ago

@apskim How would that same functionality not be achieved through internal? By using an internal base constructor or abstract internal members you are ensuring that only types defined within the same assembly can derive from the base type. private protected buys you the extra protection that other non-related types within the same assembly couldn't call those abstract members directly, but since that's your code base you could enforce that yourself. Within a team I agree that private protected would be useful as an assurance.

If I'm missing something more novel I'd be interested in seeing a gist containing an example.

alrz commented 8 years ago

@HaloFour

an IObservable<T> is not a Task<T> as the former can represent more than one result,

It doesn't matter, you can think of it as a superior abstraction, it represents one or more value, that is. In Rx you can await an IObservable<> so you'll get the latest value, whether it's the only value or not.

completely pauses at yield return and doesn't resume until the consumer invokes MoveNext

If it does so what's the point, you can return an IEnumerable<Task<T>> and iterate.

HaloFour commented 8 years ago

@alrz It matters a lot if you're trying to generate or consume more than a scalar value. :smile:

And yeah, IAsyncEnumerable<T> could be pretty easily represented by IEnumerable<Task<T>> as long as you have a convention for how the final Task<T> represents that there are no more elements, e.g. failing with NoMoreElementsException. Standard LINQ and the like would not work at all as expected, though.

Anywho these comments probably belong over at #261 since that is the proposal for async streams.

alexpolozov commented 8 years ago

@HaloFour

private protected buys you the extra protection that other non-related types within the same assembly couldn't call those abstract members directly, but since that's your code base you could enforce that yourself.

Exactly, this is an extra layer of protection for yourself and your teammates, nothing more. My general policy is that any coding guidelines you have should be expressed in the code whenever possible, not in the docs or any other form of tribal knowledge. We don't treat our team developers any differently from other developers in that regard: a compiler is your best enforcer.

Besides, private protected actually documents a member as "intended for use only in its closed hierarchy".

axel-habermaier commented 8 years ago

@HaloFour: I don't recall the specific circumstances anymore. But at one point I had the need to add a virtual protected method to a base class that can be derived externally. However, the protected method should not be visible in external assemblies, as it is an internal implementation detail of the class hierarchy. So why make it internal? That gives other people the idea that they're allowed to call that method, while they're not. So private protected (or whatever) helps the compiler to enforce correct usage. You can of course always work around it (or change the accessibility to something else), but that's besides the point.

So this is the scenario where internal does not suffice, in my opinion. What would be the alternatives? Tell people not to call that method? Add a comment that indicates that the method should not be called? Introduce a naming convention that indicates that the method should not be called? We all know how great that is going to work... (regardless of the team size, in my opinion by the way. I might confuse myself just as likely as anyone else, if I haven't looked at the code for a couple of months) For the latter, you could write analyzers that enforce this. But why go through all this hassle, when the compiler could do it just as well, using an accessibility level that .NET has always supported?

So yes, private protected doesn't gain you any new capabilities, it just allows you to express your intent more clearly. After all, you could make everything public, but we all agree that that wouldn't be a good idea. Object orientation is all about encapsulation, and private protected makes encapsulation more fain-grained in a certain -- admittedly limited -- scenario.

JohnnyBravo75 commented 8 years ago

Personally I don´t like the "with" syntax, I would prefer the following syntax to see, that it is a cloned/duplicated instance by a keyword like "dupl" or "clone". The modifiied properties can be in an object initializer syntax, like already suggested.

var cat = new Cat { Age = 10, Name = "Fluffy" }; cat = clone cat { Age = 1, Name = "FizzBuzz" };

alrz commented 8 years ago

@JohnnyBravo75 It's not a clone, it's actually a new object with following properties' values. The syntax you mentioned would imply cloning and re-initializing e.g.

cat = with cat.Clone() { Age = 1, Name = "FizzBuzz" };

based on #5676, which translates to

cat = cat.Clone();
cat.Age = 1;
cat.Name = "FizzBuzz";

On the other hand, proposed syntax is more natural, implying a new cat with following properties' values.

JohnnyBravo75 commented 8 years ago

@alrz Yes, I understoud cloning/copying with changed properties which means, they are not overwritten, but taken in the cloning process instead of the original ones, otherwise I would not need a clone keyword, I could do It with .Clone() method. But what means "new" cat then? "New" means I get a fresh new born cat, but this is not what I want. var cat = new Cat(); // new cat is born I want a cloned cat, with some modified properties during cloning, or? For me the proposed syntax is not more natural, because, assigning a cat to another means ist is the same cat where you change properties. Assigning does always (on objects) assign references, so I don´t see that this is a "new" cat. On strings this is obviuos but on objects not.

alrz commented 8 years ago

@JohnnyBravo75 These comments belong to #5172, though. Check out the example to see what happens under the hood.

bradphelan commented 8 years ago

Nested withers get ugly fast

cat = cat with { Name = cat.Name with { FirstName = "Fluffy" } }

A better notation for immutable update

cat = cat.Name.FirstName <- "Fluffy"

It is similar to the mutable version in structure

cat.Name.FirstName = "Fluffy"

and it would also be neat if you could curry it.

var fn = cat.Name.FirstName <-
cat = fn("Fluffy") 

But what about updating the property of an immutable object in an immutable list

cats = cats.Replace(cats[0], cats[0] with { Name = cats[0].Name with { FirstName = "Fluffy"}})

That's not pretty but if you can imagine composing "Withers" then you can imagine

cats = (cats with[0] with Name.FirstName) ("Fluffy")

or shorter

cats = (cats with[0].Name.FirstName) ("Fluffy")

the first part builds the "wither"

Func<ImmutableList<Cat>> fn = cats with [0] with Name.FirstName

then you can use the "wither" as a factory

cats = fn("Honey")
iam3yal commented 8 years ago

Regarding private protected, I don't think that you should do the same mistake twice and go with something like private protected because protected internal was a mistake to begin with and to add to the confusion someone thought that it's a good idea to allow both protected internal and internal protected, it isn't intuitive and unfortunately many people, especially new comers to C# think that it's actually protected and internal and like @accidentaldeveloper wrote you should create a new keyword for that but imo you should add a new syntactic sugar keyword to protected internal something like inclusive and then add a new keyword such as exclusive.

inclusive would be a syntactic sugar to protected internal whereas exclusive or segregated would mean protected and internal.

HaloFour commented 8 years ago

@eyalsk The problem with a new keyword is that none of these keywords are required to be in any specific order. static protected async internal is perfectly legal. A new keyword would be clutter that would be extremely confusing if not placed specifically where expected. internal static segregated async protected? And to think that they're considering an iterator keyword as well.

protected internal does make sense if you understand that protected and internal are two separate keywords which are applied to the member individually affecting the default accessibility of private. protected opens one door, internal opens the other, applying both means that both doors are open.

iam3yal commented 8 years ago

@HaloFour I get what you're saying and I you're right but I tend to read internal as adjective as opposed to noun so in my mind protected internal is one unit that means something else in the language even though in practice it isn't, I just like the way it reads.

On second thought maybe private protected makes more sense after all and I should start reading them as you depicted in your post.

dsyme commented 8 years ago

You might want to take a look at this AsyncSeq library in F# - http://fsprojects.github.io/FSharp.Control.AsyncSeq/. This is done in library code through F# computation expressions (no need to change the language :) )

Cancellation is fully supported and you barely need to think about it in the implementation or user model. F# async propagates cancellation tokens behind the scenes (the C# version of the F# async programming feature made cancellation tokens more explicit when they did their version of the feature, for reasons I never fully understand except that it had something to do with the WiinRT API, see http://tomasp.net/blog/csharp-async-gotchas.aspx/ and http://research.microsoft.com/apps/pubs/default.aspx?id=147194)