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: extension everything #6136

Closed
krwq opened this issue Oct 19, 2015 · 42 comments
Closed

Proposal: extension everything #6136

krwq opened this issue Oct 19, 2015 · 42 comments

Comments

@krwq
Copy link
Member

krwq commented Oct 19, 2015

Easiest explanation in the example:

class A
{
  public string ExistingMethod(int a);
  public static void ExistingStaticMethod();
}

internal /* or public */ extension class A
{
  public A(int someSpecialConstructor) : /* required */ this()
  {
    /*No need to type "this."*/ExistingMethod(someSpecialConstructor);
  }
  public void Method()
  {
    /* No access to any private members */
    /*No need to type "this."*/ExistingMethod(42);
  }
  public static void StaticMethod()
  {
    /*No need to type "A."*/ExistingStaticMethod();
  }
  public string Property
  {
    get
    {
      return "test";
    }
    set
    {
      ExistingMethod(value);
    }
  }
  // etc..
}
@svick
Copy link
Contributor

svick commented Oct 19, 2015

So, it's a unified syntax for instance extension methods (already in the language), static extension methods (#996), extension constructors, extension properties (#112) and possibly extension operators (#4945)?

@sharwell
Copy link
Member

It seems pretty straightforward. It also seems to cover everything except two items, which I'm hoping can be addressed by the eventual solution.

  1. The ability to place the extension class in a different namespace from the original
  2. The ability to use using static to bring the extension methods defined by a specific class into scope without the rest of the types/members from its namespace

@bbarry
Copy link

bbarry commented Oct 19, 2015

How about:

internal extension class AExtensions : A

Could an extension class share instance fields between extension members?

...
int callcount = 0;
public void Method() { callcount++; ... }
public int CallCount { get { return callcount; } }

    ...
    var a = new A();
    a.Method();
    a.Method();
    var b = new A();
    b.Method();
    Console.Writeline($"{a.CallCount} =?= {b.CallCount}");

@MgSam
Copy link

MgSam commented Oct 19, 2015

This is a good idea. It neatly solves the problem the design team has faced of how to add static extension methods with a decent syntax. The fact that the current extension method syntax then becomes obsolete is probably not a bad thing either- as I've always found the current syntax to be overly subtle and not particularly well designed.

@daveaglick
Copy link
Contributor

I like this a lot - neatly ties up a lot of the extension concepts floating around. What about extending interfaces (obviously without static members), or would this syntax be exclusively for classes? I.e.:

interface IA
{
  string ExistingMethod(int a);
}

internal /* or public */ extension interface IA  /// or "class IA" ???
{
  public void Method()
  {
    ExistingMethod(42);
  }
  public string Property
  {
    get
    {
      return "test";
    }
    set
    {
      ExistingMethod(value);
    }
  }
  // etc..
}

Perhaps the declaration syntax proposed by @bbarry (internal extension class AExtensions : IA) would work better in this case because it makes it obvious the extension class is an actual class (unless it's intended to be some new kind of thing?) and allows inheriting/implementing from either a class or an interface. Though it wouldn't actually be implementing the interface since we're not defining the interface members themselves - which could make the overloaded use of : confusing.

What about internal class AExtensions extends A and internal class AExtensions extends IA?

@HaloFour
Copy link

I don't know, I like the explicitness of the current extension method syntax as accepting the instance via a parameter. With this syntax the compiler would need to add the this parameter changing the signature which may be confusing, particularly with reflection scenarios.

I also have concerns about the concept of extension properties (and events) given that they're not just accessor methods but also the member metadata which can't be amended on the target class.

Both relatively minor points. Beyond that it does seem like a decent way to provide extension members.

Thoughts as to how the class would be emitted by the compiler? What is the name of the extension class?

internal static class `A.extension` {
  [Extension] // property to indicate a constructor?
  public static A `.ext_ctor`(int someSpecialConstructor)
  {
    A tmp = new A();
    tmp.ExistingMethod(someSpecialConstructor);
    return tmp;
  }

  [Extension]
  public static void Method(A `this`)
  {
    `this`.ExistingMethod(42);
  }

  [Extension(typeof(A))] // property to indicate extended type?
  public static void StaticMethod()
  {
    A.ExistingStaticMethod();
  }

  [Extension] // property to indicate a property?
  public static string get_Property(A `this`)
  {
    return "test";
  }

  [Extension]
  public static void set_Property(A `this`, string value)
  {
    `this`.ExistingMethod(value);
  }
}

@MgSam
Copy link

MgSam commented Oct 19, 2015

@HaloFour

On method rewriting- this is already done extensively with async and iterator methods, so it doesn't seem like a big jump to apply a similar concept elsewhere.

I'm not sure I follow the second point correctly, but an implementation of extension properties could use the ConditionalWeakTable to store state for a type externally.

@HaloFour
Copy link

@MgSam

Rewriting the method body, sure, but the signature is always left intact.

My second point isn't about extension "fields" or state, which is a set of problems unto itself. It's that while "extension properties" might fake syntax of feeling like a property that they can't be properties where reflection metadata is concerned making them useless in many of the scenarios used for properties like serialization or data binding. That may be a worthwhile tradeoff to have the syntax but it could also be quite confusing to have something that smells like a property not be able to be used like a property.

@bbarry
Copy link

bbarry commented Oct 19, 2015

What if lowering was done like this; given this extension class:

internal extension class AExtensions : A
{
  private int field = 0;
  public A(int someSpecialConstructor) : this()
  {
    ExistingMethod(someSpecialConstructor);
  }
  public void Method()
  {
    ++field;
    ExistingMethod(field);
  }
  public static void StaticMethod()
  {
    ExistingStaticMethod();
  }
  public string Property
  {
    get
    {
      return "test";
    }
    set
    {
      ExistingMethod(value);
    }
  }
}

Generated code looks something like this:

#line 1 
internal static class AExtensions 
{
#line hidden
  private static ConditionalWeakTable<A, AExtensions.InternalType> dictionary = new ConditionalWeakTable<A, AExtensions.InternalType>();

  private static AExtensions.InternalType Load(A a, InternalType constructed = null)
  {
    return dictionary.GetValue(a, a1 => constructed ?? new AExtensions.InternalType(a1));
  }

  private class InternalType
  {
    public WeakReference _a;

    public InternalType(A a) 
    {
      _a = new WeakReference(a);
    }

#line 3
    private int field = 0;
    public InternalType(A a, int someSpecialConstructor) : this(a)
    {
      _a.Target.ExistingMethod(someSpecialConstructor);
    }
    public void Method()
    {
      ++field;
      _a.Target.ExistingMethod(field);
    }
    public static void StaticMethod()
    {
      A.ExistingStaticMethod();
    }
    public string Property
    {
      get
      {
        return "test";
      }
      set
      {
        _a.Target.ExistingMethod(value);
      }
    }
  }
#line hidden

  [ExtensionConstructor]
  public static A __ctor(int someSpecialConstructor)
  {
    var a = new A();
    Load(a, new AExtensions.InternalType(a, someSpecialConstructor));
    return a;
  }

  [ExtensionMethod]
  public static void Method(this A a)
  {
    Load(a).Method();
  }

  [ExtensionStaticMethod(typeof(A))] // perhaps this one doesn't get lifted like this?
  public static void StaticMethod()
  {
    InternalType.StaticMethod();
  }

  [ExtensionPropertyGetter]
  public static string get_Property(this A a)
  {
    return Load(a).Property;
  }

  [ExtensionPropertySetter]
  public static string set_Property(this A a, string value)
  {
    return Load(a).Property = value;
  }
}

I'm not convinced extension properties are a good thing either and feel the same for extension constructors. I think static extension methods and extension operators have merit in the "let's play nicer with functional programming language style" meta-goal but I am not sure of the others. "My" suggested syntax wasn't mine to begin with either, @chrisaut suggested it in #112 (comment) (I suspect this issue is a dupe).

@bbarry
Copy link

bbarry commented Oct 20, 2015

I think explicit cast operators to interface adapters could be defined as well:

public abstract class A 
{
  public abstract void ExistingMethod(int i);
}
public interface IB
{
  void ExistingMethod(int i);
  void NewMethod();
}

internal extension class AExtensions : A, IB
{
    void NewMethod() { }
}

now any class that derives from A can be explicitly cast ((IB)obj) to IB.

#line 1 
internal static class AExtensions 
{
#line hidden
  private static ConditionalWeakTable<A, AExtensions.InternalType> dictionary = new ConditionalWeakTable<A, AExtensions.InternalType>();

  private static AExtensions.InternalType Load(A a, InternalType constructed = null)
  {
    return dictionary.GetValue(a, a1 => constructed ?? new AExtensions.InternalType(a1));
  }

  private class InternalType
  {
    public WeakReference _a;

    public InternalType(A a) 
    {
      _a = new WeakReference(a);
    }

#line 3
    public void NewMethod() { }
  }
#line hidden

  public class AdapterIB : IB // struct?
  {
    private readonly A _a;
    public AdapterIB(A a) { _a = a; }
    public void ExistingMethod(int i) => _a.ExistingMethod(i);
    public void NewMethod() => AExtensions.NewMethod();
  }

  [ExtensionMethod]
  public static void NewMethod(this A a)
  {
    Load(a).Method();
  }

  [ExtensionOperator]
  public static explicit operator IB(this A a)
  {
    return new AdapterIB(a);
  }
}

@dfkeenan
Copy link

I have been thinking along the similar lines. But was thinking more like:

internal /* or public */ extension StringExtensions : String
{

  // etc..
}

Where the "inheritance" bit is what defines what the extension is targeting. That way you can define it where you like (namespace etc.) and only include it where necessary via a using as you currently do for extension methods.

Not sure I am keen on being able to add state to existing types this way though.

@MgSam
Copy link

MgSam commented Oct 20, 2015

@HaloFour These are good points, but I think both seem minor in the scheme of things. I wouldn't expect an extension property (or anything else) to be any more available by type reflection than an extension method is today, and the difference could theoretically be highlighted in the IDE by colorization (although at present the colorization provided to C# by VS is still stuck in the 1990s). It seems like it would be pretty unusual that you'd be actually reflecting over the extension class itself.

I think adding state to existing types is an important use case. For example, WPF's attached properties are very useful but at the same time extremely kludgy. A language-based solution would have been much preferable. As we know extension properties already almost made it into the language but for failing to meet this use case. We should also take a cue from the JavaScript/TypeScript world, where bolting new behavior and state onto types is fairly common.

@gafter gafter changed the title Proposition: extension class Proposal: extension everything Oct 20, 2015
@gafter
Copy link
Member

gafter commented Oct 20, 2015

We've been thinking about this and calling it "extension everything", so I updated the title.

@dfkeenan
Copy link

@MgSam

WPF is an Entity Component System which has been designed with a special use case in mind (Styling, property inheritance etc.). Each DependencyObject is a collection of it's own state. Having object state stored in a static collection in another class seems like GC/performance issue just waiting to happen.

JavaScript objects are also a collection of there own state. JavaScript allow bolting-on because it is a dynamically typed language with that design in mind. C# already has this covered with dynamic dispatch and types like ExpandoObject. TypeScript is a super-set of JavaScript and uses same runtime so same comments are applicable here.

I think coming up with a one size fits all (or most) solution for this would be difficult.

@gafter
Copy link
Member

gafter commented Oct 21, 2015

@dfkeenan Additional state (e.g. a field) is something we are not likely to support in a feature like this.

@orthoxerox
Copy link
Contributor

Any chance of supporting extension virtual/abstract methods? They might have an unwanted side effect of breaking your code when you reference an assembly with them, and they might require CLR changes, since there's no place in the vtable left for them, but they are or so useful.

@MgSam
Copy link

MgSam commented Oct 21, 2015

@dfkeenan Take a look at the ConditionalWeakTable class I mentioned earlier. It holds only weak references, so when your object is GCed so is any data associated with it. In fact it was specifically designed for this sort of use case when they built the DLR.

@MgSam
Copy link

MgSam commented Oct 21, 2015

@gafter It seems like extension properties would be mostly useless if you cannot store additional state with them.

@sharwell
Copy link
Member

I would be in favor of supporting additional state as part of an extension class for any reference type. Support for this functionality has been available in the CLR since .NET 4.0 as part of supporting various dynamic languages. It's not always the most efficient approach, but it's reasonable to think for certain applications it would make implementing new functionality much easier.

@gafter
Copy link
Member

gafter commented Oct 21, 2015

@orthoxerox Not without the CLR team's help. We have higher priority things we'd ask for.

@MgSam @sharwell There are lots of things (computed but not stored) extension properties are useful for. The use of ConditionalWeakTable slows down the GC substantially when it is used, so we're not ready to base a language feature on it. On the other hand, you could use a ConditionalWeakTable in the implementation of an extension property to get the behavior even without language support... if you're willing to pay the price.

@grwGeo
Copy link

grwGeo commented Oct 22, 2015

@krwq , @sharwell Please take a look at #3357 for a discussion on type extensibility.

@MgSam
Copy link

MgSam commented Oct 22, 2015

@gafter Is it not true that extension properties were previously designed and jettisoned because they couldn't meet the needs of WPF's attached properties? Clearly the team at some point thought it was important to have them store state.

ConditionalWeakTable was created for the DLR, right? The DLR and associated machinery was added to the language despite the performance penalties. I understand that an extension property with state might make GC slower than a real property, but like any feature it has a cost/benefit tradeoff. I think the language should provide that choice to the developer, not prematurely try to optimize it away.

Yes, it's true you could hack together your own extension property that stores state. But you can already do that today with extension methods. If extension properties, constructors, etc. add nothing over existing extension methods save a slightly nicer invocation syntax then it hardly seems worth the considerable design effort to add them into the language.

@HaloFour
Copy link

@MgSam I heard it was an issue of syntax, going with something that looked like property syntax except further decorated. A generic extension indexer ended up mixing three kinds of brackets in one construct. WPF was always going to use its own state mechanism so it wouldn't have mattered if the language included a concept of extension state and if the language tried to force it most of the functionality of attached dependency properties wouldn't have worked anyway.

@gafter
Copy link
Member

gafter commented Oct 22, 2015

@MgSam ConditionalWeakTable is a normal class built on the preexisting mechanism DependentHandle, which is what causes the performance issue. When DependentHandles exist in the heap, they slow down garbage-collection, and the more of them exist the larger the impact on garbage collection. Consequently any language feature that depends on them would affect not just the performance of a library that uses them directly, but any code in which such a library is used. It is a "peanut butter" cost that seems to slow down everything. It is the kind of thing we prefer not to introduce in the language.

@MgSam
Copy link

MgSam commented Oct 22, 2015

Interesting. Thanks for the explanation.

Though I do think that underlying implementation details should probably be secondary considerations when deciding the merits of a feature (if there's a will, there's a way). That being said, IMHO I think extension everything is near the bottom of the "features I'd really like to see" list, and not worth doing at all if additional state is not part of the deal.

@alrz
Copy link
Member

alrz commented Oct 22, 2015

@gafter I wonder why there are dynamic types in a totally statically typed language. That is a horrible idea IMO. I can't look at the IL generated for dynamic types because that would be dangerous and there's a chance of heart attack. I would prefer #3555 for sure.

@gafter
Copy link
Member

gafter commented Oct 22, 2015

@gafter I wonder why there are dynamic types in a totally statically typed language. That is a horrible idea IMO. I can't look at the IL generated for dynamic types because that would be dangerous and there's a chance of heart attack. I would prefer #3555 for sure.

@alrz Your timing could be better. Couldn't you have wondered that some six or seven years ago? You would have saved us an enormous amount of work 😉

dynamic is for interacting with dynamic languages that are built on the CLR. #3555 is for making some common data structures easier to work with.

@dfkeenan
Copy link

How do you scope the use of the extension? What if 2 libraries extend the same class? What if they have member name clashes? or both extensions implement the same Interface? Although I don't think the CLR could currently support extensions that implement Interfaces. Though that would be quite cool. I thought was one of the nifty features of Swift.

After asking these questions I started to wonder how do you disambiguate extension properties/indexers? Would the generated methods just be available as static methods with nice names. i.e. a property called Total would generate a method GetTotal. Would probably be fine with some special rules for code completion list generation.

@grwGeo
Copy link

grwGeo commented Oct 28, 2015

@dfkeenan Very valid question. Conflicts as such are not the issue. Resolving them is the issue. Such conflicts can arise with extension methods too, when you reference two libraries/namespaces that might define an extension with the same name and signature on the same type.

When that happens the compiler does not complain until you call the extension methods, in which case there will be ambiguity. In that case, as a workaround, you could call the extension method via its static class and pass the extended type instance as a parameter and resolve the ambiguity.
When it comes to full extension types we are faced with the heart of the issue and a deeper way to resolve it would be required. It looks like the actual name of the extending class (the one that extends the original) might be required after all.

A syntax proposal could then be (see also #3357):

 public class MyExtendingClass : ISomeInteface1, ISomeInteface2 extensionof MyOriginalClass
 {    
       private int _myExField = 0;
       public int MyExProperty { get { return _myExField; } }
       //Other extension members
 }

(extensionof would be a keyword having similar position to where in generics)
In case some other library extends the same type with the same member name, then some casting could be used to resolve the ambiguity:

 MyOriginalClass myInstance = new MyOriginalClass();
((MyExtendingClass)myInstance).MyExProperty…

against:

((SomeOtherExtendingClass)myInstance).MyExProperty… 

Casting of course would not be needed when there is no ambiguity.

Something interesting to keep in mind is that there might be room for inheritance in the extending class, i.e. the extending class might be able to inherit from some other class (not only interfaces). Which means that multiple inheritance might be achievable with extension classes given that there is clear separation between the hierarchy of the original class and the hierarchy of the extension.

@dfkeenan
Copy link

"Casting" might be a good way to disambiguate. Not sure I like the syntax for the extension itself though. Maybe take a page out of the generics book and us the <> syntax which reads as "of". So something like:

public interface ITotal
{
    int TotalLength { get; }
}

public extension ThatsALotOfString<IEnumerable<string>> : ITotal
{
      public int TotalLength { get { return this.Sum( s => s.Length);  } }
}

or maybe:

public extension ThatsALotOfString<IEnumerable<T>> : ITotal
    where T : string
{
      public int TotalLength { get { return this.Sum( s => s.Length);  } }
}

@grwGeo
Copy link

grwGeo commented Oct 29, 2015

I am not sure I understand what class is being extended.

@dfkeenan
Copy link

Well in my example it's IEnumerable<string>. Why limit extensions to just classes? I think you should be able to extend any type just like you currently can with extension methods.

@Thaina
Copy link

Thaina commented Jan 16, 2016

I like the idea of go style to make extension of everything and let go back to use pure struct along with interface

But I would go against making class for extend things

Instead we should extend everything with the extension method. May tweak some syntax for extension constructor. But all should be inside static class as the same. Or no class at all (Yes, back to the old function only like C++)

@alrz
Copy link
Member

alrz commented Jan 29, 2016

Since this is the main issue for extensions I'd like to note extension class is redundant and also it should support generics and constraints. See #8127 for an example.

I'm curious to see how extension constructors would look like, perhaps they have to return an object? If so can we cover #6788 with it?

@vbcodec
Copy link

vbcodec commented Feb 13, 2016

Copy of Partial ?

@Joe4evr
Copy link

Joe4evr commented Feb 13, 2016

How would you be able to do partial if you don't own the source of the thing you want to extend?

@vbcodec
Copy link

vbcodec commented Feb 13, 2016

@Joe4evr
Class A is defined in code. There is no mention about extending compiled classes. For compiled classes there are already extensions, which provide nearly the same possibilities.

@axel-habermaier
Copy link
Contributor

@vbcodec: "nearly" is the important word: There are only extension methods, but no extension properties, extension events, extension static methods, or extension constructors.

@gafter
Copy link
Member

gafter commented Apr 28, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


This may be close enough to dotnet/csharplang#192 that discussion can continue there.

@gafter gafter closed this as completed Apr 28, 2017
@gafter
Copy link
Member

gafter commented Apr 28, 2017

I removed the comment "I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this.". In fact, the LDM is considering doing this. See dotnet/csharplang#192

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