333fred commented 2 years ago

Discussed in https://github.com/dotnet/csharplang/discussions/5496

^{Originally posted by MadsTorgersen November 30, 2021}

Extensions

[x] Proposed
[ ] Prototype: Not Started
[ ] Implementation: Not Started
[ ] Specification: https://github.com/dotnet/csharplang/blob/main/proposals/extensions.md

LDM Meetings

hez2010 commented 2 years ago

I prefer using of or something else to distinguish the type being extended and interfaces:

public extension Foo of int : IA, IB, IC, ...
{
    ...
}

Otherwise it will be too confusing if you are extending an interface:

public extension Foo : IA, IB, IC { }

vs

public extension Foo of IA : IB, IC { }

of can be relatively safely made as a keyword since it's neither a verb nor a noun, so almost nobody would choose of as an identifier.

HaloFour commented 2 years ago

I'm curious as to how the team weighs the relative benefits between "roles" and "extension implementation". It feels that without some additional effort in the runtime the two are somewhat incompatible with each other, so if those differences can't be reconciled which of the features might the team lean towards?

Personally, I find extension implementation much more exciting than roles, but that's just my opinion.

FaustVX commented 2 years ago

@hez2010 Maybe, instead of of, for would be a better name for the keyword as it's already a keyword.

public extension Foo for IA : IB, IC { }

333fred commented 2 years ago

I'm curious as to how the team weighs the relative benefits between "roles" and "extension implementation". It feels that without some additional effort in the runtime the two are somewhat incompatible with each other, so if those differences can't be reconciled which of the features might the team lean towards?

Who gave you an early preview of my notes? They're up now, discussion at https://github.com/dotnet/csharplang/discussions/5500.

sab39 commented 2 years ago

Here's a scenario that will be great fun to try to accommodate in the design:

interface IFoo { }
interface IBar { }
class Thing { }
public extension FooThing for Thing : IFoo { }
public extension BarThing for Thing : IBar { }
void Frob<T>(T t) where T : IFoo, IBar { }

Frob(new Thing());

On an unrelated bikeshedding note, what about using the existing reserved keywords explicit and implicit as modifiers, rather than treating roles and extensions as completely separate things? An extension is, more or less, just a role that gets applied implicitly based on type rather than needing to be explicitly named in the declaration. Using implicit role as the syntax would spell that correspondence out more (no pun intended) explicitly?

CyrusNajmabadi commented 2 years ago

@sab39 Given, as you've mentioned, how similar these two concepts are. I too am looking for a good syntactic way to convey that similarity, with a clear way to do indicate in which way they differ. Thanks for the explicit/implicit idea, definitely something we'll consider!

TahirAhmadov commented 2 years ago

I'm not sure if I should re-post my comments from the discussion here? In short, I think the extensions and especially interface "adapters" make sense, but roles don't. The main motivation example - DataObject - is an anti-pattern, IMO; it runs into expensive runtime changes; and causes confusion - now you have to keep in mind when looking at an identifier in a "type receiving context" if it's a type or a role.

Here's a scenario that will be great fun to try to accommodate in the design:
interface IFoo { }
interface IBar { }
class Thing { }
public extension FooThing for Thing : IFoo { }
public extension BarThing for Thing : IBar { }
void Frob<T>(T t) where T : IFoo, IBar { }

Frob(new Thing());
This is complicated, but doable using current constraints of the framework. An anonymous type can be generated:
class <mangled>Thing_IFoo_IBar : IFoo, IBar
{
internal <mangled>Thing_IFoo_IBar(Thing thing) { this._thing = thing; }
readonly Thing _thing;

void IFoo.Foo() { ... } // these member(s) are copied from, or call into, FooThing void IBar.Bar() { ... } // these member(s) are copied from, or call into, BarThing } Frob(new Thing_IFoo_IBar(new Thing()));


The same can be done for generic types, etc. Yes, it's complicated, but unlike roles, it's very possible.

CyrusNajmabadi commented 2 years ago

The main motivation example

This was just one example. It's not the main motivation. We discussed in the LDM that there were definitely plenty of scenarios where you'd still want adapters in a strongly typed way that would be sensible.

sab39 commented 2 years ago

@TahirAhmadov That works, more or less, for the specific example I gave, but what if Frob<T> had other constraints like where T : class or where T : struct or where T : new() or where T : SomeBaseClass? What about if it were Frob<T, T2> where T2 : T? In general it's not actually possible to generate an anonymous type that can meet all possible constraints that T would meet and also implement IFoo and IBar. This suggests to me that it's not possible to fully support this scenario without runtime assistance.

TahirAhmadov commented 2 years ago

The main motivation example

This was just one example. It's not the main motivation. We discussed in the LDM that there were definitely plenty of scenarios where you'd still want adapters in a strongly typed way that would be sensible.

If it's not the main motivation, surely it shouldn't be the one discussed in the OP, should it?

CyrusNajmabadi commented 2 years ago

The OP is simply showing a demonstration. This is a broad topic and we need to spend a ton more time on it prior to even getting close to a place where we could write something up that was fully fleshed out and chock full of examples and whatnot.

TahirAhmadov commented 2 years ago

@TahirAhmadov That works, more or less, for the specific example I gave, but what if Frob<T> had other constraints like where T : class or where T : struct or where T : new() or where T : SomeBaseClass? What about if it were Frob<T, T2> where T2 : T? In general it's not actually possible to generate an anonymous type that can meet all possible constraints that T would meet and also implement IFoo and IBar. This suggests to me that it's not possible to fully support this scenario without runtime assistance.

The where T : class constraint simply rejects Thing extensions if Thing doesn't satisfy the constraints. The where T : new() rejects all extensions outright. where T : SomeBaseClass also rejects extensions of Thing because it's a different type. Frob<T, T2> where T2 : T is completely irrelevant.

BhaaLseN commented 2 years ago

Back with .NET Framework, I've often ran into situations where i wanted a Math.Clamp<T>(T value, T min, T max), Math.Max(TimeSpan val1, TimeSpan val2) or Path.GetRelativePath(...) for discoverability; but there was no way for me to get this done. Same with string.Contains(string, StringComparison) etc. except as instance extension (which is somewhat taken care of by extension methods though.) Nowadays most are in there since either .NET Core or .NET 5/6, but it would feel more natural to simply extend the existing classes (with a very high-up namespace inside the project, so its most likely in scope all the time; or with a global using for example) when another one of those situations comes up. At least compare to MathEx, MathUtilities, PathHelpers etc. which often aren't as obvious.

The only thing I don't quite get is why we need two keywords here, role and extension. They mean different things if you talk about them, sure, but does this actually matter once you write the code? I'd assume they'll be lowered to virtually the same thing during compilation after all, and I can practically hear the "what's the difference" question coming when I present this to my team during a knowledge transfer meeting.

TahirAhmadov commented 2 years ago

The OP is simply showing a demonstration. This is a broad topic and we need to spend a ton more time on it prior to even getting close to a place where we could write something up that was fully fleshed out and chock full of examples and whatnot.

That's the thing, it would be very interesting to see an example which would demonstrate how roles make something worthwhile possible or significantly easier, before effort is spent on prototypes, implementation planning, etc.

CyrusNajmabadi commented 2 years ago

That's fine. It's something we're working on at this moment '-). The point was raised and was something we intend to get to and write more on. I def don't want us to get the impression that it's just for that. Thanks!

orthoxerox commented 2 years ago

Roles feel like they need a validator method, something that is invoked to by the "implicit conversion" to ensure that the underlying object can fill in that role. I'm not even sure the conversion should be implicit. I'm sure it will be annoying to do stuff like public Customer Customer => (Customer)this["Customer"]; over and over again, but I also want to be able to say if (payload is Order order) { ....

sab39 commented 2 years ago

Hmm, that almost makes it sound like you want Extension DUs...

orthoxerox commented 2 years ago

I don't want them to be a DU per se, it's more similar to getting an object from some API and casting it to the expected type. Right now the roles work more like dynamic instead.

vladd commented 2 years ago

@orthoxerox F# has a feature Partial Active Patterns which looks somewhat like your idea.

hez2010 commented 2 years ago

This is complicated, but doable using current constraints of the framework. An anonymous type can be generated:
class <mangled>Thing_IFoo_IBar : IFoo, IBar
{
  internal <mangled>Thing_IFoo_IBar(Thing thing) { this._thing = thing; }
  readonly Thing _thing;

  void IFoo.Foo() { ... } // these member(s) are copied from, or call into, FooThing
  void IBar.Bar() { ... } // these member(s) are copied from, or call into, BarThing
}
Frob(new <mangled>Thing_IFoo_IBar(new Thing()));
The same can be done for generic types, etc. Yes, it's complicated, but unlike roles, it's very possible.

C# isn't the only language on CoreCLR, without runtime support how would you expect roles to be defined and used in other languages? Other languages don't recognize the mangled anonymous class.

TahirAhmadov commented 2 years ago

C# isn't the only language on CoreCLR, without runtime support how would you expect roles to be defined and used in other languages? Other languages don't recognize the mangled anonymous class.

The pseudocode I wrote was specifically for extensions, not roles. In any case, though, the mangled anonymous type is generated at the call site, not at the extension declaration site. Specifically because there can be multiple permutations of extensions (or roles - ignoring the fact that I don't like the idea of roles/shapes), these machinations have to be performed when all the information is available: what interfaces are "adapted", etc. Also, regardless of the language, the extension will have to be added somehow to the metadata; the easiest way would be using a class with some special attributes. The other languages can decide whether to implement this feature - in which case they can interpret these attributes like C# does; otherwise, it becomes a class, probably a static one, which they can use in an "old school way". The same is true for existing extension methods. Further, even when it's a simple scenario, for interface "adaptation" to work, the easiest way is again, an anonymous type:

class Thing { }
interface IFoo { void Foo(); }
extension FooThing: Thing, IFoo { void Foo() { ... } }
void Frob(IFoo foo) { }
// this line:
Frob(new Thing());
// is compiled to this:
class <mangled>Thing_IFoo : IFoo
{
  internal <mangled>Thing_IFoo(Thing thing) { this._thing = thing; }
  readonly Thing _thing;

  void IFoo.Foo() { ... } // these member(s) are copied from, or call into, FooThing
}
Frob(new <mangled>Thing_IFoo(new Thing()));

Thaina commented 2 years ago

I also want to voice that I wish there would be some keyword being reused instead of casting new keyword role and extension. Or at least create only one and use implicit/explicit as above

Or implicit class possible?

Aside from that I have nothing against, and fully support this issue

hez2010 commented 2 years ago

instead of casting new keyword role and extension. Or at least create only one and use implicit/explicit as above

Or implicit class possible?

Keywords can be introduced as contextual keywords so it can be made not to introduce breaking changes.

Thaina commented 2 years ago

@hez2010 I know there is no breaking change but it still should be the last option to introduce any new keyword. If there would be any possible for composite or reuse then we should

333fred commented 2 years ago

I found the idea of implicit and explicit very interesting and forwarded your comment to our working group @sab39, thanks for the suggestion!

iam3yal commented 2 years ago

I don't get why roles need an implicit and explicit modifiers, can't they be applied based on the context? what does it mean to have these modifiers? why treat them more or less the same and not exactly the same where the only difference is context? I get the you're trying out different ideas but merge these concepts needs to be core principle the way I think about is similar to aggregation vs composition where aggregation is an extension of existing type and composition is a wrapper the only difference is what they user want them to be based on context and not how they were constructed I don't think we want to end up with a situation where "I can do this when it's explicit but not when it's implicit or vice-versa" but maybe I'm misunderstanding why we need to have different rules for these two concepts, it's not clear whether they are similar or identical features yet but based on the OP I think they are either identical or similar to the point where it can be confusing to grasp why we are speaking about two different concepts.

I'll just copy/paste my comment from the other post so something like this:

// Customer.cs
namespace Data;

public extension Customer : DataObject // Wrapper type
{
    public string Name => this["Name"].AsString();
    public string Address => this["Address"].AsString();
    public IEnumerable<Order> Orders => this["Orders"].AsEnumerable();
}

// JsonDataObject.cs
namespace Data;

using JsonLibrary;

public extension JsonDataObject : DataObject // Extension type
{
    public string ToJson() { … this … }
    public static T FromJson<T>(string json) { … }
}

// Program.cs / Main method
using Data;
using Data.JsonDataObject; // Importing a specific extension type
using Data.*; // Importing all extensions types in the namespace Data

var customer = customer.FromJson<Customer>(args[0]);
WriteLine(customer.ToJson());

Reinms commented 2 years ago

Would this be allowed under roles?

role Foo<T> : T
    where T : ISomeInterface
{
}

or would we be forced to directly extend the interface and bring in boxing conversions all over the place as we implicitly cast back and forth in a generic function?

TahirAhmadov commented 2 years ago

Thinking about it, I imagine this happening:

class Thing { }
interface IFoo { void Foo(); }
// the following line
public extension FooThing: Thing, IFoo { void Foo() { ... } }
// is compiled to:
// these attributes are once per assembly, similar to NRT attributes
class ExtensionTypeAttribute { public ExtensionTypeAttribute(params Type[] types) { ... } ... }
class ExtensionInstanceMemberAttribute {  }
class ExtensionStaticMemberAttribute {  }
// the actual extension becomes:
[ExtensionType(typeof(Thing), typeof(IFoo))]
public static class FooThing
{
  [ExtensionInstanceMember]
  public static void Foo(Thing @this) { ... }
}

void Frob(IFoo foo) { }

// this line:
Frob(new Thing());
// is compiled to this:
class <mangled>Thing_IFoo : IFoo
{
  internal <mangled>Thing_IFoo(Thing thing) { this._thing = thing; }
  readonly Thing _thing;

  void IFoo.Foo() { FooThing.Foo(this._thing); }
}
Frob(new <mangled>Thing_IFoo(new Thing()));

0x0737 commented 2 years ago

@eyalalonn

I don't get why roles need an implicit and explicit modifiers...

Why unify roles and extensions? Roles are intended to be used only in specific scenarios, extensions - in pretty much any scenarios. Roles are declared as roles to prevent popping in where it's not appropriate. If you want to apply a role to all conforming types implicitly, you declare an extension for existing role. Extensions are often coupled with the other types in a namespace, so auto-importing them with the rest of the namespace (like existing extension methods) makes sense. Also, extension keyword is more weird for type aliases than role. I think it is better to leave role and extension as they are.

iam3yal commented 2 years ago

@0x0737

Why unify roles and extensions? Roles are intended to be used only in specific scenarios, extensions - in pretty much any scenarios. Roles are declared as roles to prevent popping in where it's not appropriate. If you want to apply a role to all conforming types implicitly, you declare an extension for existing role. Extensions are often coupled with the other types in a namespace, so auto-importing them with the rest of the namespace (like existing extension methods) makes sense. Also, extension keyword is more weird for type aliases than role. I think it is better to leave role and extension as they are.

In the OP Mads stated the following:

Extensions are just roles that apply automatically to their underlying type when brought into scope with a using directive.

In my opinion stating that roles are intended to be used only in specific scenarios is incorrect because like Mads stated above extensions are roles, you're constructing an extension by creating a role, they are just applied differently at least that's what I'm getting at but if this is the case then in my opinion we don't really need more than a single keyword and definitely something like implicit and explicit although they might used optionally to limit the scope or something so when implicit is used then the extension automatically brought into scope but I'm not sure whether this is the intention here.

0x0737 commented 2 years ago

@eyalalonn Yes, I understand that extensions are roles under the hood. They just have different use cases on the language side, like classes/structs and records for example. So, different keywords make sense from my point of view

iam3yal commented 2 years ago

@0x0737 I wasn't referring to the implementation but I respect your opinion.

sirinath commented 2 years ago

Great to see this proposal. I think the starting point would be to think about runtime changes to accommodate this and possibly future proposals and extensions so the runtime itself will not be a constrain and show stopper or limiting factor.

hez2010 commented 2 years ago

@TahirAhmadov

Thinking about it, I imagine this happening:

class Thing { }
interface IFoo { void Foo(); }
// the following line
public extension FooThing: Thing, IFoo { void Foo() { ... } }
// is compiled to:
// these attributes are once per assembly, similar to NRT attributes
class ExtensionTypeAttribute { public ExtensionTypeAttribute(params Type[] types) { ... } ... }
class ExtensionInstanceMemberAttribute {  }
class ExtensionStaticMemberAttribute {  }
// the actual extension becomes:
[ExtensionType(typeof(Thing), typeof(IFoo))]
public static class FooThing
{
  [ExtensionInstanceMember]
  public static void Foo(Thing @this) { ... }
}

void Frob(IFoo foo) { }

// this line:
Frob(new Thing());
// is compiled to this:
class <mangled>Thing_IFoo : IFoo
{
  internal <mangled>Thing_IFoo(Thing thing) { this._thing = thing; }
  readonly Thing _thing;

  void IFoo.Foo() { FooThing.Foo(this._thing); }
}
Frob(new <mangled>Thing_IFoo(new Thing()));

Then how would you like to deal with structs and ref structs? Ref structs can't be saved in fields and saving structs in class fields would lead to boxing and introduce a lot of overhead.

class <mangled>SomeClass
{
    Span<T> s; // error
}
class <mangled>SomeClass
{
    SomeStruct s; // boxing to heap
}

This feature cannot be done perfectly without support from the runtime.

TahirAhmadov commented 2 years ago

Then how would you like to deal with structs and ref structs? Ref structs can't be saved in fields and saving structs in class fields would lead to boxing and introduce a lot of overhead.
class <mangled>SomeClass
{
    Span<T> s; // error
}
class <mangled>SomeClass
{
    SomeStruct s; // boxing to heap
}
This feature cannot be done perfectly without support from the runtime.

The solution is easy - ref structs cannot be extended with an interface adapter. Not only it's a good way to avoid having to deal with the problem, but it also doesn't really make sense. Ref structs is a feature aimed at extremely high performance code; things like extensions and interface adapters are not pertinent in that scenario, IMO. Re. regular structs, it's already the case that implemented interface methods cannot modify the original struct, because of boxing; so this behavior is consistent with the solution I proposed - the struct is boxed and extensions become de-facto "readonly".

interface IStruct
{
    void Set(int x);
}
struct Struct : IStruct
{
    public int A;
    public void Set(int x) => this.A = x;
}
var st = new Struct();
IStruct ist = st;
ist.Set(123);
Console.WriteLine(st.A); // outputs 0

Note that these limitations only apply to interface adaptation, and not using the regular "extension" features of the new extension syntax, like extension methods and properties; those just become glorified static methods and act the same as current "this" extension methods.

Reinms commented 2 years ago

Then how would you like to deal with structs and ref structs? Ref structs can't be saved in fields and saving structs in class fields would lead to boxing and introduce a lot of overhead.
class <mangled>SomeClass
{
    Span<T> s; // error
}
class <mangled>SomeClass
{
    SomeStruct s; // boxing to heap
}
This feature cannot be done perfectly without support from the runtime.
The solution is easy - ref structs cannot be extended with an interface adapter. Not only it's a good way to avoid having to deal with the problem, but it also doesn't really make sense. Ref structs is a feature aimed at extremely high performance code; things like extensions and interface adapters are not pertinent in that scenario, IMO. Re. regular structs, it's already the case that implemented interface methods cannot modify the original struct, because of boxing; so this behavior is consistent with the solution I proposed - the struct is boxed and extensions become de-facto "readonly".
interface IStruct
{
  void Set(int x);
}
struct Struct : IStruct
{
  public int A;
  public void Set(int x) => this.A = x;
}
var st = new Struct();
IStruct ist = st;
ist.Set(123);
Console.WriteLine(st.A); // outputs 0
Note that these limitations only apply to interface adaptation, and not using the regular "extension" features of the new extension syntax, like extension methods and properties; those just become glorified static methods and act the same as current "this" extension methods.

Here is the problem with that approach: Ref structs aren't purely for extremely high performance, they are for high performance with relative safety and ease of use, before ref structs all high performance code would be working directly with pointers and unmanaged memory. Now, they make it possible to get that high performance without needing to throw away safety and readability.

Every time a restriction like that is added the divide between high performance code and ordinary code becomes wider and wider, and it's already far too wide (see: ref struct records, pointer records, ref struct generics, pointer generics, ref struct interfaces, tuples, ref structs in async methods, pointers in async methods, and many many more). The answer to a question like "how do I make this function run faster?" should not involve a ground up redesign to not use a large number of great language features because if it does, people who are in even slightly performance sensitive situations are going to just not use those features by default (Aka: premature optimization) due to the pain involved in removing usage of those features.

TahirAhmadov commented 2 years ago

Ref structs aren't purely for extremely high performance, they are for high performance with relative safety and ease of use, before ref structs all high performance code would be working directly with pointers and unmanaged memory. Now, they make it possible to get that high performance without needing to throw away safety and readability.

Trust me, I'm the first person in any group to advocate for writing high performance code. I hate the expression "premature optimization" - it's that expression which is responsible for needing a quad core CPU to just launch an OS. Having said that, it's important to understand the difference between "high performance" and "extreme high performance". If you take most of the code running in production today, it can be drastically optimized without sacrificing any other considerations. We're talking about repetitive string allocations due to bad string concat code, disastrous database queries, outright memory leaks, etc. Once all these optimizations are performed, it can be described as "performant". WRT pointers and ref structs, we're talking about something beyond that optimization. Now, we're looking at saving nanoseconds of heap allocations of smallish structs. This is "extremely performant", and it isn't always needed.

Every time a restriction like that is added the divide between high performance code and ordinary code becomes wider and wider, and it's already far too wide

I understand that concern, but unfortunately, in this case, our hands are tied, even if we had the agreement of CLR team to make any changes we ask of them. In the above scenario, the Frob(IFoo foo) method expects an object which implements IFoo, and that object has to be on the heap because interface implementation relies on v-table. These are core OOP constructs and cannot be modified; some kind of heap allocation will have to happen in any case - so "extreme high performance" is out the window right away. However, even if we said, OK we'll sacrifice performance to have the extension interface adapter feature, we're still not out of the woods. I can imagine maybe storing a pointer to that struct or ref struct and putting it on the heap, and telling GC how to handle this dependency type. But I'm sure the moment you propose that, there will be all kinds of people knowledgeable with inner workings of these things explaining how it's expensive or impossible. At the end of the day, in the best case, you'll be making very expensive changes, and the result will be weak - basically you get syntax consistency. I'm not sure the cost/benefit there is good.

Reinms commented 2 years ago

Ref structs aren't purely for extremely high performance, they are for high performance with relative safety and ease of use, before ref structs all high performance code would be working directly with pointers and unmanaged memory. Now, they make it possible to get that high performance without needing to throw away safety and readability.

Trust me, I'm the first person in any group to advocate for writing high performance code. I hate the expression "premature optimization" - it's that expression which is responsible for needing a quad core CPU to just launch an OS. Having said that, it's important to understand the difference between "high performance" and "extreme high performance". If you take most of the code running in production today, it can be drastically optimized without sacrificing any other considerations. We're talking about repetitive string allocations due to bad string concat code, disastrous database queries, outright memory leaks, etc. Once all these optimizations are performed, it can be described as "performant". WRT pointers and ref structs, we're talking about something beyond that optimization. Now, we're looking at saving nanoseconds of heap allocations of smallish structs. This is "extremely performant", and it isn't always needed.

Every time a restriction like that is added the divide between high performance code and ordinary code becomes wider and wider, and it's already far too wide

I understand that concern, but unfortunately, in this case, our hands are tied, even if we had the agreement of CLR team to make any changes we ask of them. In the above scenario, the Frob(IFoo foo) method expects an object which implements IFoo, and that object has to be on the heap because interface implementation relies on v-table. These are core OOP constructs and cannot be modified; some kind of heap allocation will have to happen in any case - so "extreme high performance" is out the window right away. However, even if we said, OK we'll sacrifice performance to have the extension interface adapter feature, we're still not out of the woods. I can imagine maybe storing a pointer to that struct or ref struct and putting it on the heap, and telling GC how to handle this dependency type. But I'm sure the moment you propose that, there will be all kinds of people knowledgeable with inner workings of these things explaining how it's expensive or impossible. At the end of the day, in the best case, you'll be making very expensive changes, and the result will be weak - basically you get syntax consistency. I'm not sure the cost/benefit there is good.

In the past though, restrictions like these don't end up being focused on the actual cause of the issue. See ref struct records. The entire concept was tossed because the record couldn't implement an interface, despite the fact that all the behaviours of a record could have been implemented directly in them. Specifically blocking a few problematic cases is fine but historically a few problematic cases tends to bring in a huge amount of collateral.

fabianoliver commented 2 years ago

A very interesting proposal. Is there anything this proposal allows do to that would currently not be possible, though?

I've just gone through the specs today, so there's a good chance I'm missing something major. My current reading is that manually written adapters are already able to do everything roles can, just with a little more boilerplate. Except for maybe applicability - i.e. applying the role implicitly through import, rather than creating adapters explicitly.

If it does indeed boil down to mostly convenience, I would be reluctant to see such a feature implemented. It is likely a big work item, adds complexity to the language itself. For all the many cases where it could improve code quality, I'd expect to see an equal number of cases where it makes code a whole lot harder to read and follow. I don't even want to count how often I've seen extensions abused to implement important functionality, spread across many different files far away from the actual class definition, making it near impossible to follow outside of IDEs.

And last buy not last, while implicit creation/application of adapters would be convenient, I think it could easily lead to confusion and easy-to-miss bugs in places. Let's say,

// Does this work, does this not work.. who knows, let's hope I remembered to import the extension, because the compiler won't be able to help me out here, so I'll probably find out at runtime!
if(customer is IPerson person)
  Huzzah();

Or

using MyCustomerExtensions;

// I guess this works
if(customer is IPerson person)
  Huzzah();

// If above succeeds, it might be easy to assume this does too.. but I guess it wouldn't
if(typeof(IPerson).IsAssignableFrom(customer.GetType())
  Huzzah();

Or

public extension CustomerAsPerson1 : Customer, IPerson
{
    string IPerson.FullName => "Jay";
}

public extension CustomerAsPerson2 : Customer, IPerson
{
    string IPerson.FullName => "Joe";
}

object something = GetCustomer();
if(something is IPerson person)
  // What happens here? Is it going to be disallowed using conflicting extensions? Would it need to be handled at runtime somehow?
  Huzzah();

As mentioned though, I'm not firmly familiar with this proposal, maybe I'm missing out on something and this can't happen, or there are other benefits that make up for the potential downsides? Is there anything this proposal allows us do to that is entirely impossible at present?

TahirAhmadov commented 2 years ago

If it does indeed boil down to mostly convenience, I would be reluctant to see such a feature implemented. It is likely a big work item, adds complexity to the language itself. For all the many cases where it could improve code quality, I'd expect to see an equal number of cases where it makes code a whole lot harder to read and follow. I don't even want to count how often I've seen extensions abused to implement important functionality, spread across many different files far away from the actual class definition, making it near impossible to follow outside of IDEs.

It's not only convenience - it adds extension properties, events, and static extension methods, properties and events. This is a huge deal. Yes, features can be abused, but I don't think this is an example of a feature where it "drives" people to make mistakes; coders have to actively want to create these extensions to cause problems. This isn't a "pit of fail" type of feature, IMO - if you think otherwise, please explain.

And last buy not last, while implicit creation/application of adapters would be convenient, I think it could easily lead to confusion and easy-to-miss bugs in places. Let's say,

// Does this work, does this not work.. who knows, let's hope I remembered to import the extension, because the compiler won't be able to help me out here, so I'll probably find out at runtime!
if(customer is IPerson person)
  Huzzah();

Or

using MyCustomerExtensions;

// I guess this works
if(customer is IPerson person)
  Huzzah();

// If above succeeds, it might be easy to assume this does too.. but I guess it wouldn't
if(typeof(IPerson).IsAssignableFrom(customer.GetType())
  Huzzah();

Great points. Given that is is a run-time check, and extensions are fundamentally a compile-time feature, I think all of these shouldn't be allowed. The only way to use interface adapters should be compile-time knowledge.

Or

public extension CustomerAsPerson1 : Customer, IPerson
{
    string IPerson.FullName => "Jay";
}

public extension CustomerAsPerson2 : Customer, IPerson
{
    string IPerson.FullName => "Joe";
}

object something = GetCustomer();
if(something is IPerson person)
  // What happens here? Is it going to be disallowed using conflicting extensions? Would it need to be handled at runtime somehow?
  Huzzah();

It should act just like it does today with existing extension methods - produce the error "The call is ambiguous between the following methods or properties". PS. Correction: technically, your example would not work because is wouldn't work at all in this case. But in the following example, it should produce the error I mentioned above:

public extension CustomerAsPerson1 : Customer, IPerson
{
    string IPerson.FullName => "Jay";
}

public extension CustomerAsPerson2 : Customer, IPerson
{
    string IPerson.FullName => "Joe";
}

IPerson person = GetCustomer(); // error: The interface adaptation is ambiguous between the following extensions: CustomerAsPerson1 and CustomerAsPerson2

0x0737 commented 2 years ago

@fabianoliver

My current reading is that manually written adapters are already able to do everything roles can, just with a little more boilerplate

Manually written adapters change object identity, roles do not, so it's possible to use them as type aliases too

fabianoliver commented 2 years ago

@TahirAhmadov

It's not only convenience - it adds extension properties, events, and static extension methods, properties and events. This is a huge deal.

Thanks for the clarification. For the sake of the debate, I'll play devil's advocate for a bit:

We can already associate "extension"-like data & events with instances of any type, say through conditional weak tables. So from a functional point of view, I'm not quite convinced (yet?) these points enable us to do something that's entirely impossible at present?

In terms of syntax - no debate there, the proposal would without doubt make things a lot easier and arguably more concise, if used sensibly.

Great points. Given that is is a run-time check, and extensions are fundamentally a compile-time feature, I think all of these shouldn't be allowed. The only way to use interface adapters should be compile-time knowledge.

Interesting idea - I think this could lead to very odd situations though. Let's say,

using MyCustomerExtensions;

void DoStuff(IPerson person)
{
  if(person is not IPerson person)
    throw new Exception("What is going on?!"); // Breaking change?
}

Customer customer = GetCustomer();
DoStuff(customer);

If i understand the proposal & your idea correctly, we'd end up throwing here. Which is think would be extremely counter-intuitive - and in fact probably even a big breaking change, given we currently could not hit this branch at all (nullability aside).

Realistically, even if we were to accept such a breaking change - given the ubiquitous use of "is" and reflection, this would seem like an almost inevitable source of bugs, and we'd end up with "Schrodinger's interface" of sorts

@0x0737

@fabianoliver

My current reading is that manually written adapters are already able to do everything roles can, just with a little more boilerplate

Manually written adapters change object identity, roles do not, so it's possible to use them as type aliases too

That is a great point indeed (and raises the slightly adjacent question, should we be allowed to extend sealed types?)

On the flipside, unlike manual adapters - based on the discussion above, runtime behaviour of extension types may need a little further clarification.

CyrusNajmabadi commented 2 years ago

I'm not quite convinced (yet?) these points enable us to do something that's entirely impossible at present?

This is true of most of our features. The goal is not to make the impossible possible. It's to make it pleasant and easy to do these powerful things.

TahirAhmadov commented 2 years ago

We can already associate "extension"-like data & events with instances of any type, say through conditional weak tables. So from a functional point of view, I'm not quite convinced (yet?) these points enable us to do something that's entirely impossible at present?

In terms of syntax - no debate there, the proposal would without doubt make things a lot easier and arguably more concise, if used sensibly.

If I understand correctly, extension events do not give you the ability to add a state to a foreign type; I imagine that these would have to be explicit events - and the add/remove accessors would either do what you described (weak tables) or perhaps transform the handler in some way and attach it to another existing event, whatever is appropriate. The compiler can emit the "weak table" code, implemented using some hidden ConcurrentDictionary and whatnot, and allow implicit extension events. That would be a cool feature - I'm not sure if that is being considered. Extensions are fundamentally syntax sugar; everything is already possible with static helper classes and manually written interface adapter classes; but those techniques are ugly, long, etc.

Great points. Given that is is a run-time check, and extensions are fundamentally a compile-time feature, I think all of these shouldn't be allowed. The only way to use interface adapters should be compile-time knowledge.

Interesting idea - I think this could lead to very odd situations though. Let's say,
using MyCustomerExtensions;

void DoStuff(IPerson person)
{
  if(person is not IPerson person)
    throw new Exception("What is going on?!"); // Breaking change?
}

Customer customer = GetCustomer();
DoStuff(customer);
If i understand the proposal & your idea correctly, we'd end up throwing here. Which is think would be extremely counter-intuitive - and in fact probably even a big breaking change, given we currently could not hit this branch at all (nullability aside).

No, there would be no throw. The actual instance received by DoStuff is an adapter hidden class which does implement IPerson, and has a reference to Customer. That is check will work correctly. What I meant by "compiler knowledge" is that the compiler sees that the extension is in scope; it sees you're passing a Customer to something expecting IPerson; and emits the adapter class and all that automatically. This is not the same as what is being discussed elsewhere - ref struct interface constraints being implemented by the compiler mapping method calls without using the interface v-table. (In case that's what you thought it meant.)

0x0737 commented 2 years ago

@fabianoliver

That is a great point indeed (and raises the slightly adjacent question, should we be allowed to extend sealed types?)

On the flipside, unlike manual adapters - based on the discussion above, runtime behaviour of extension types may need a little further clarification.

From what I have understood, the runtime behavior of roles is exactly the same as of classic adapters except for reference comparisons.

Regarding sealed types - I don't see any reason why that should be banned, since we already can extend them with old "this" methods and/or use plain static methods. So my opinion is that disallowing to extend them will only introduce artificial inconvenience. Interface adaption for sealed types is probably OK too, since we can create wrappers for them already and extensions/roles are just a more efficient and convenient way to do that

TahirAhmadov commented 2 years ago

Following up on something that has come up in a different thread - will extensions be allowed to define operators on the extended type? Also, currently overload resolution favors native methods to extension methods, if there is a conflict. I guess that should continue being the case for all extension members, as well as interface adaptations? With roles, this is more complicated. I think it should be the opposite - roles' "augmentations" should override all native members/interfaces.

333fred commented 2 years ago

will extensions be allowed to define operators on the extended type?

Yes.

Also, currently overload resolution favors native methods to extension methods, if there is a conflict. I guess that should continue being the case for all extension members, as well as interface adaptations?

Depends on whether you're talking about a role or an extension. Since a role has to be manually added, there's more room to play around with overload resolution there. For extensions, yes, native functionality will still need to be preferred.

TahirAhmadov commented 2 years ago

Also, currently overload resolution favors native methods to extension methods, if there is a conflict. I guess that should continue being the case for all extension members, as well as interface adaptations?

Depends on whether you're talking about a role or an extension. Since a role has to be manually added, there's more room to play around with overload resolution there. For extensions, yes, native functionality will still need to be preferred.

Do you think it makes sense to add a warning for these (in addition to defaulting to native)? I'm talking about extension syntax only, not current extension methods. This will be a new syntax, so it's not a breaking change. This can probably help people catch bugs earlier.

333fred commented 2 years ago

Also, currently overload resolution favors native methods to extension methods, if there is a conflict. I guess that should continue being the case for all extension members, as well as interface adaptations?

Depends on whether you're talking about a role or an extension. Since a role has to be manually added, there's more room to play around with overload resolution there. For extensions, yes, native functionality will still need to be preferred.

Do you think it makes sense to add a warning for these (in addition to defaulting to native)? I'm talking about extension syntax only, not current extension methods. This will be a new syntax, so it's not a breaking change. This can probably help people catch bugs earlier.

Maybe? Seems like it could be reasonable.

CyrusNajmabadi commented 2 years ago

Do you think it makes sense to add a warning for these

I'd need to see examples. And, to me, unless almost certainly an issue, i would prefer warnings be in the domain of analyzrs.

--

Given that we don't have any warnings for extension methods (that i'm aware of) in the Roslyn compiler, despite them being 15+ years old, i don't really imagine a deep need for warnings for tehse extensions either.

dotnet / csharplang

[Proposal]: Extensions #5497

Discussed in https://github.com/dotnet/csharplang/discussions/5496

Extensions

LDM Meetings