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

Champion "Extension function members" #192

Open
gafter opened this issue Feb 26, 2017 · 48 comments
Open

Champion "Extension function members" #192

gafter opened this issue Feb 26, 2017 · 48 comments

Comments

@gafter
Copy link
Member

@gafter gafter commented Feb 26, 2017

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

See also dotnet/roslyn#11159

@gafter gafter added this to the X.0 candidate milestone Feb 26, 2017
@Thaina
Copy link

@Thaina Thaina commented Feb 27, 2017

Almost agree and totally support with a bit of arguments and concerns

  • Name of this feature make it not sure if this will be able to extend field or not, I would disagree to extend fields

  • Extension struct should always pass by reference. Same behaviour as normal implementation of struct. This solve the problem of passing struct by ref for extension method altogether

  • How can we find duplicate operator? When it cause function conflict error it will be hard to find, unlike named member that could go to reference

  • Would this syntax will be able to extend enum ?

  • Is this include extension to implement interface ?

  • We don't need new keyword extension if we could permit static class to extend anything. Reusing keyword and it already intuitive to have static class hold extension method

/// static class cannot extend anything before so it not breaking any previous code
public static class Ext : MyStruct
{
}
@Bartmax
Copy link

@Bartmax Bartmax commented Jun 7, 2017

I think I'm late to the party and forgive my ignorance, but what about just supporting partial keyword for non-partial classes ?

@orthoxerox
Copy link

@orthoxerox orthoxerox commented Jun 7, 2017

@Bartmax partial is for classes in the same assembly. It simply combines several pieces of the same type during compilation. Extensions are for extending arbitrary classes, including those from external compiled assemblies.

@Bartmax
Copy link

@Bartmax Bartmax commented Jun 7, 2017

i know that, but that doesn't mean it can be "reworked" to allow classes from other assemblies hence creating extensions for arbitrary classes.

@alrz
Copy link
Contributor

@alrz alrz commented Jul 6, 2017

I'd suggest the following syntax for extension declarations,

internal extension [class] StringExtensions for String { }
internal extension [class] GenericExtensions<T> for T where T : class { }

I believe the name should be optional for convenience (e.g. private nested extension declarations don't really need to be named):

private extension [for] String { }
private extension<T> [for] T where T : class { }

Similarly, instead of base list, we could use implement to plug interface/shape/traits to types,

implement TraitT for String { }
implement<T> FooT for T where T : BarT { }

This has various advantages like segregating impls so that they do not show up on all instances. Explicit implementations could have the same effect in extension declarations, but I think it's good to separate these concerns (auxiliary methods vs trait implementations).

@ghost
Copy link

@ghost ghost commented Jul 18, 2017

Please consider this feature for the nearest releases. It would make life so much easier.

@hacklex
Copy link

@hacklex hacklex commented Aug 25, 2017

I think it would be good to have the ability to write extensions for multiple classes in a single context. For example, if you need to cache reflection objects (especially when you're emitting something), sharing said cache would probably be a good idea, and creating an additional static class for that sole purpose would feel somewhat awkward.

Also, I think many would benefit from extensions existing in a non-global context (consider private static void ExtMethod(this SomeClass x) being inside a non-static class).

@paulomorgado
Copy link

@paulomorgado paulomorgado commented Aug 26, 2017

@Bartmax, there is no such thing as partial classes. There are partial class definitions and it's a source code feature not an assembly feature (@orthoxerox).

@gmengano
Copy link

@gmengano gmengano commented Oct 25, 2017

X.0 or 8.0?
(See A preview of c# 8 with Mads Torgersen)

@UweKeim
Copy link

@UweKeim UweKeim commented Oct 25, 2017

@Joe4evr
Copy link

@Joe4evr Joe4evr commented Oct 25, 2017

Well, it's 100% in the X.0 Candidate milestone. And you can click it if you don't know what that means.

@IanKemp
Copy link

@IanKemp IanKemp commented Nov 8, 2017

@Joe4evr That milestone has a release date of January 1 2100 - I think most of us would like to see this feature in a version of C# that's released before we're all dead...

@JVimes
Copy link

@JVimes JVimes commented Nov 29, 2017

@paulomorgado and @orthoxerox, I'm sure @Bartmax is not suggesting using partial class definitions. He's suggesting using a similar syntax to theirs, but for extension members. It's a great idea:

public extension class ClassName
{
    // Extension members of all types go here
    // Don't need to pollute parameter lists with "this ClassName foo"
}
@ivi-hamiti
Copy link

@ivi-hamiti ivi-hamiti commented Dec 1, 2017

@JVimes I totally agree with your proposed syntax so not to introduce anymore new keyword like the example with the for keyword. But what about the cases you are implementing extension methods for types that implement a certain interfaces? Event the naming would be misleading. Like we are doing and extension class for IEnumerable and you have to do something like the following:

public extension class IEnumerable<T>
{
    // Extension members of all types go here
    // Don't need to pollute parameter lists with "this ClassName foo"
}

Which breaks the naming conventions for C#. Based on your approach i would go with a syntax like this

public extension class ExtensionName : ClassName 
{
    // Extension members of all types go here
    // Don't need to pollute parameter lists with "this ClassName foo"
}

So it should support the same syntax as class inheritance, generics, etc. The only thing with this approach is that it may be misleading to that user, thinking he should implement the interface or abstract class (as it looks like you are extending/implementing some type.

@michael-hawker
Copy link

@michael-hawker michael-hawker commented Dec 17, 2017

So, would this proposal cover being able to add events as an extension to an existing class as well?

@jnm2
Copy link
Collaborator

@jnm2 jnm2 commented Dec 18, 2017

dotnet/roslyn#11159 (comment):

Limitations:

  • Events not permitted (at first)
@leo60228
Copy link

@leo60228 leo60228 commented Mar 11, 2018

I assume this would support interfaces. This would be really neat, since you could make a mixin like this (with the syntax specified by ivi-hamiti):

interface IMyMixin {}

public extension class MyMixinImpl : IMyMixin {
    // mixin stuff here
}
@AnorZaken
Copy link

@AnorZaken AnorZaken commented Apr 10, 2019

So how about keeping it familiar, but still different enough to not cause confusion - instead of the normal single colon : for inheritance, this could use double colons ::
public static class MyExtensions :: ExtendedClass
Or has anyone suggested this already?

@PathogenDavid
Copy link

@PathogenDavid PathogenDavid commented Apr 10, 2019

While it isn't commonly used (and wouldn't make sense in that context), it's worth noting that :: is already in use for namespace alias qualifiers.

Personally, I'd rather see the introduction of the extension keyword in place of static for extension types. It puts more emphasis on the fact that the type plays a special role compared to others.

@matthew25187
Copy link

@matthew25187 matthew25187 commented May 8, 2019

Rather than having a special type of extensions class, why not modify the existing syntax for extension methods? For example,

public static class DayOfWeekExtensions
{
  // Methods
  public static bool IsWeekend() extends DayOfWeek
  {
     return this == DayOfWeek.Saturday || this == DayOfWeek.Sunday;
  }

  // Properties
  public static bool IsWeekend extends DayOfWeek
  {
     get => this == DayOfWeek.Saturday || this == DayOfWeek.Sunday;
  }
}

The extends keyword defines the context of the this keyword within the body of the extension. This allows for a single extensions class to extend the capabilities of multiple types, as do existing extension classes that currently implement extension methods. Extension methods could also overload methods from the extended type, so long as the method signature is unique. If the extended type is later modified to add an overload that is already defined as an extension, the extension overload would have to be flagged as a compilation error.

You could also extend interfaces, where the this keyword refers to an instance of the interface. For example,

public static class EnumerableExtensions
{
    public static int CountNulls<T>() extends IEnumerable<T> where T : class
    {
        return this.Count(item => item == null);
    }
}

When using generics, as in this example, the generic type arguments would be limited to those employed by the extended type so the compiler would know their context at the point the extension was called.

This strategy could also be applied to other constructs such as indexers.

// Contrived example!
namespace ExtensionsExample
{
    public struct FirstName
    {
        private string _name;

        public FirstName(string name)
        {
            _name = name;
        }
    }

    public struct LastName : IEquatable<LastName>
    {
        private string _name;

        public LastName(string name)
        {
            _name = name;
        }

        public bool Equals(LastName other)
        {
            return other._name == _name;
        }
    }

    public class PersonName
    {
        public FirstName FirstName {get; set;}
        public LastName LastName {get; set;}
    }

    public class PersonNameCollection : CollectionBase
    {
        // Collection stuff.
    }

    public class Person
    {
        public PersonNameCollection Names {get;}
    }

    public static class PersonExtensions
    {
        public static PersonName this[LastName index] extends PersonNameCollection
        {
            get => this.Cast<PersonName>().SingleOrDefault(item => item.LastName.Equals(index));
        }
    }
}
@matthew25187
Copy link

@matthew25187 matthew25187 commented May 8, 2019

How would a class being sealed affect the ability of extensions to add capabilities to it? Would the sealed keyword prohibit the definition of extensions on the class?

@brunoais
Copy link

@brunoais brunoais commented May 9, 2019

sealed only affects the class. An extension is, in its essence, syntactic sugar for a wrapper around the original class and to make as if the original class had those methods. It never changes that class itself.

@AnorZaken
Copy link

@AnorZaken AnorZaken commented May 17, 2019

When using generics, as in this example, the generic type arguments would be limited to those employed by the extended type so the compiler would know their context at the point the extension was called.

I would never accept the loss of defining my own generic types in an extension.
That's a serious regression of functionality ...or I'm misunderstanding you.

@willmotil
Copy link

@willmotil willmotil commented May 25, 2019

Since you guys are discussing this in relation to stringbuilder as well.
You may like to see a practical StringBuilder wrapper class that i made and keep updating that i use in practice gif at the bottom.
In order to solve problems with the regular StringBuilder, primarily with 2 things that it falls short in both relate to Garbage Collections depending on the context Massive amounts for even simple things.

  1. it's Insert method.
  2. c# conversion of numeric variables into strings to the StringBuilder in a dynamic context.

Both of these can generate a huge amount of garbage in a realtime situation with massive massive amounts of collections.

Operator Overloading is the least of its problems however it is a garbage collection problem if used.

(I know you guys can do it better then me.)

Since you are all pro's here i will not remake a new project just to remove the MonoGame specific methods which will error for straight c# use. They can just be commented out there are only a couple small append methods.

The code to illustrate and the output below that in a gif.

        MgStringBuilder msg = new MgStringBuilder("Hello World ");
        int counter = 0;
        protected override void QuickDrawSpriteBatch(GameTime gameTime)
        {
            msg.Clear();
            counter++;
            //msg = " *operator* "; // fine 
            //msg += " Hello " + "World "; // works but creates garbage due to operator overloading limitations.
            msg.Append(" *Append* ");
            msg.Append("Hello ").Append("World ").Append(counter); // i can make this work with no garbage.
            
            // print out
            //Console.WriteLine(msg);

            // in my case i draw
            spriteBatch.DrawString(Gu.currentFont, msg, new Vector2(300, 10), Color.Wheat);
        }

My Wrapper.

https://github.com/willmotil/MonoGameUtilityClasses/blob/master/MgStringBuilder.cs

gcstringbuilder01

This is actually running at over 2000 fps my gif program slowed it down when i was recording though.

@tullo-x86
Copy link

@tullo-x86 tullo-x86 commented May 28, 2019

@willmotil, I think you might have posted your comment on the wrong issue...

@ddobrev
Copy link

@ddobrev ddobrev commented Jul 12, 2019

Is there any actual work ongoing? When is this going to be available?

@Trojaner
Copy link

@Trojaner Trojaner commented Jul 12, 2019

@ddobrev it is in the X.0 candidate milestone, which means it's on the candidate list for a future version. I assume no work on this is currently done as C# 8 is still not finished.

@michael-hawker
Copy link

@michael-hawker michael-hawker commented Jul 22, 2019

Yeah, I was sad to see previews coming out that didn't have this feature. Been waiting for this one!

@lobster2012-user
Copy link

@lobster2012-user lobster2012-user commented Nov 12, 2019

Many of fsharp's features have been ported to csharp.
Unfortunately, this did not affect custom operators, I hope this will be fixed.
custom operator |> csharp

@ddobrev
Copy link

@ddobrev ddobrev commented Apr 15, 2020

C# 8 seems finished and this isn't in. When is it going to?

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Apr 15, 2020

@ddobrev This is no release that this feature is scheduled for.

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Apr 15, 2020

@ddobrev Your comment isn't helpful or constructive. Please try to stay on topic. I understand you very much want this feature. However, this is one of thousands of features that people would like and we're prioritizing based on lots of pieces of data to determine which ones we should ship.

Importantly, many features we do are not at all sugar (though many are as well). Much of the last few releases has been driving work that is enormously important for scalability and performance (especially for games, server environments, and mobile devices).

Hopefully in the future you'll find our picks are more what you want. But that won't always be the case. Have a good day :)

@munael
Copy link

@munael munael commented Apr 17, 2020

Any definition doc to read for this proposal? It mentions "Extension Everything" under the roslyn repo. But that's

  1. Under a different repo, and is closed.
  2. Uses different language from this issue.

What are "Extension function members"? Is there a post missing or is missing a log from an older issue?

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Apr 17, 2020

@narfanar there is no proposal yet:

image

@ChayimFriedman2
Copy link

@ChayimFriedman2 ChayimFriedman2 commented Apr 29, 2020

Personally, I discourage field extensions. This is against the philosophy of adding a helper to the class. If you need to store data, that's not a helper - that data structure. And data structure should be implemented by inheritance, probably.

Also that will face many technical challenges, which will cause either poor performance or introducing changes in the CLR (or both) because the only ways (do you have others?) to implement this feature is a) implement a dictionary for the field (performing poorly) or b) somehow extends the CLR so objects can have additional fields. But properties (and indexers, etc.) are welcome.

Interfaces are also good (i.e. implementing interface through extensions) since interfaces indicates an ability of the type. For instance, be serialized, format itself, etc..

@NetMage
Copy link
Contributor

@NetMage NetMage commented May 28, 2020

@CyrusNajmabadi Is Mads no longer working?

@burnss9
Copy link

@burnss9 burnss9 commented Aug 27, 2020

Hard to determine the current proposal syntax from this thread and I'm not certain where to find it elsewhere. I've seen some mentions of proposals where they keyword static isn't present. I'm fairly uneducated on the backend of how this all works but currently it looks like the IL extension methods are static, if this would go unchanged the [MethodImpl(MethodImplOptions.Synchronized)] attribute would be very unintuitive on extension methods/properties. Seems the attribute currently would lock "statically" not for each class instance (makes sense given the static method) but if the static keyword is removed that could be pretty confusing.

@quinmars
Copy link

@quinmars quinmars commented Aug 27, 2020

The use of MethodImplOptions.Synchronized is discouraged anyway:

Locking on the instance or on the type, as with the Synchronized flag, is not recommended for public types, because code other than your own can take locks on public types and instances. This might cause deadlocks or other synchronization problems.

@HaloFour HaloFour mentioned this issue Sep 20, 2020
1 of 4 tasks complete
@michael-hawker
Copy link

@michael-hawker michael-hawker commented Oct 23, 2020

I'm not sure if it'd be part of this proposal or a new feature request. But I just encountered a scenario where I wanted to have an extension that was using a protected method, but I'd only be calling the extension method from within the scope of the parent class which has access to the protected method.

It'd be great to be able to flag this type of scenario so the compiler would let me access the protected sub-class methods as long as I'm only using the extension method from a place where I'd have access to those compared to just anywhere.

E.g. In XAML, when extending and writing a custom Control, we inherit from the Control class which has a protected GetTemplateChild method. However this is generalized and not strongly typed. We wanted to create a typed version which would encapsulate the cast and check for null if we thought it was a required component.

We do this as a private method currently, but it is a general function applicable to anyone inheriting from Control and would have been great to generalize and re-use code more effectively with an extension method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.