[Proposal]: Compromise design for extensions #8917
Replies: 85 comments
-
So, we'll have old extension methods, new extension methods, and new extension types?
That is putting it very, very mildly. |
Beta Was this translation helpful? Give feedback.
-
No one has demonstrated it is straightforward. |
Beta Was this translation helpful? Give feedback.
-
I kind of agree, I feel that this compromise design leaves things in a worse state. Now we'd have three syntaxes, with one intended to replace the other but only necessary in the more complicated scenarios. I wonder what percentage of those existing extension methods actually fall into that category. I'm going to throw some half-thought-out spaghetti using constraint syntax at the wall: public extension Enumerable for IEnumerable {
// simple extension method
public IEnumerable<T> OfType<T>() { ... }
// static helper method
public static IEnumerable<T> Empty<T>() { ... }
// generic extension constraints
public IEnumerable<T> Where<T>(Func<T, bool> predicate)
where this : IEnumerable<T> { ... }
public IOrderedEnumerable<TSource> ThenBy<TSource, TKey>(Func<TSource, TKey> selector)
where this : IOrderedEnumerable<TSource> { ... }
} Honestly not sure that it really differs from the compromise proposal, but it feels like it's trying to address it via specialization rather than two flavors of extension methods. |
Beta Was this translation helpful? Give feedback.
-
[ExtensionMethods("Enumerable")] // static class name
public extension EnumerableExtension<T> for IEnumerable<T>
{
public IEnumerable<T> Where(Func<T, bool> predicate) { ... }
public IOrderedEnumerable<T> OrderBy<TKey>(Func<T, TKey> selector) { ... }
[RenameTypeArgument("T", "TSource")] // if we want to preserve the old type argument name
public IOrderedEnumerable<T> OrderBy<TKey>(Func<T, TKey> selector) { ... }
}
// generated
public static partial class Enumerable
{
// insert extension generics in the beginning of method generics automatically
public static IEnumerable<T> Where<T>(this IEnumerable<T> This, Func<T, bool> predicate)
=> ((EnumerableExtension<T>)This).Where(predicate); // and omit them when calling the new extension
// insert before method's generics
public static IOrderedEnumerable<T> OrderBy<T, TKey>(this IEnumerable<T> This, Func<T, TKey> selector)
=> ((EnumerableExtension<T>)This).OrderBy<TKey>(selector); // in this case, the generic of the extension type is removed,
// but additional generic parameters are kept
// with the rename
public static IOrderedEnumerable<TSource> OrderBy<TSource, TKey>(this IEnumerable<TSource> This, Func<TSource, TKey> selector)
=> ((EnumerableExtension<TSource>)This).OrderBy<TKey>(selector);
} |
Beta Was this translation helpful? Give feedback.
-
The problem is the generics, as I expect a lot of people would assume that would emit the class I, personally, think that flattening the generics is probably the cleanest way to go, but I can understand this concern. However, supporting |
Beta Was this translation helpful? Give feedback.
-
How can they possibly assume that a genetic class will be generated, if generic classes cannot house extension methods? We're talking novice/ school levels of knowledge. I doubt it's the same group who maintains LINQ and other libraries. 🤣 I'm not sure I understand the rest of your comment, so I'll refrain from responding in any way. |
Beta Was this translation helpful? Give feedback.
-
Because the developer is writing what looks like a generic type, not a static class, so the limitations specific to static classes don't necessarily apply.
I'm agreeing that moving the generic type argument from the extension to the method, as you suggested, is probably the cleanest/easiest to emit the extension types to remain binary compatible. Additionally, if you can write multiple public partial extension Enumerable<T> for IEnumerable<T> {
public IEnumerable<T> Where(Func<T, bool> predicate) { ... }
}
public partial extension Enumerable<T> for IOrderedEnumerable<T> {
public IOrderedEnumerable<T> ThenBy<TKey>(Func<T, TKey> selector) { ... }
}
// emits
public partial static class Enumerable {
public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate) { ... }
public static IOrderedEnumerable<T> ThenBy<T, TKey>(this IOrderedEnumerable<T> source, Func<T, TKey> selector) { ... }
} |
Beta Was this translation helpful? Give feedback.
-
The limitations apply because the attribute and the SG it triggers are clearly and unequivocally for generating "old extension methods", and those have to be in a non-generic class. Just a little comprehension of the context quickly clarifies the intent. Besides, they can always inspect the generated code, and existing test suites would either fail to compile or fail to execute if there were any issues.
If I understand correctly, you mean the language should do it as part of the feature? There are a few major issues with this.
Really though, I don't even think an SG is necessary. If we adopt a very clear direction of "all extension stuff going forward will be done using new extension types", then the only extension methods we need to worry about are the ones currently existing in the ecosystem; there is no need to create new old-style extension methods after extension types ship. Consumption of new extensions can only happen in new code, which means binary compat is a non-issue. And for existing old extension methods, the code fixer can handle the "transition" with the "convert-and-retain-a-redirect" feature. |
Beta Was this translation helpful? Give feedback.
-
Basically, we already have: public static class Enumerable
{
public static IEnumerable<T> Where<T>(this IEnumerable<T> This, Func<T, bool> predicate) { ... }
// ...
} After code fixer: public extension EnumerableExtension<T> for IEnumerable<T>
{
public IEnumerable<T> Where(Func<T, bool> predicate) { ... }
// ...
}
public static class Enumerable
{
public static IEnumerable<T> Where<T>(this IEnumerable<T> This, Func<T, bool> predicate)
=> ((EnumerableExtension<T>)This).Where(predicate);
// ...
} No extension feature back compat necessary, no SG necessary. Just the original design and a smart code fixer. |
Beta Was this translation helpful? Give feedback.
-
Yes, I strongly believe that we're far from the point of punting on this and evoking source generators. Extensions are a core language feature, and as long as this new thing is also called "extensions" it seems obvious that the language should strive to bridge the gap between them and provide a built-in mechanism to safely evolve them. C# is a 22 year old language, and extensions are a 17 year old language feature. We can't just throw out that enormous amount of existing code or relegate it to the bin of deprecation, regardless of how "clean" a potential replacement could be. |
Beta Was this translation helpful? Give feedback.
-
Frankly, the stuff that I wrote above is so obvious to me, that I suspect all of it is known to LDT and there are reasons that escape me why they're going down the road they are going. |
Beta Was this translation helpful? Give feedback.
-
BTW: https://github.com/microsoft/referencesource/blob/master/System.Core/System/Linq/Enumerable.cs They're not using |
Beta Was this translation helpful? Give feedback.
-
Curious why that is? I assume interfaces implemented on a partial declaration would be only applied to that target. IMO the name should be only there for disambiguation, which probably require some new syntax to be used with operators/properties as well as conversion to an interface. However that turned out to be, you can still disambiguate by name since each extension can only implement an interface once on each target. On top of that, I wouldn't find it surprising if the language emit |
Beta Was this translation helpful? Give feedback.
-
Well, it at least becomes a conversation. I agree, it doesn't preclude extension implementation, and there are options for what the language could do to support it with a set of heterogeneous partial extensions, but that depends on whether the team is up for that.
I could see it slightly surprising/confusing, but I agree that most people probably wouldn't even notice. I almost wonder if there's a better way to describe that the extension target is generic without having to declare the extension itself as generic, as a way to better clarify it to the developer. |
Beta Was this translation helpful? Give feedback.
-
I think that's somehow essential with this because it would mean that type parameters are strictly not part of the type definition, allowing |
Beta Was this translation helpful? Give feedback.
-
btw, a core issue I have is you describing things as quirks, or edge cases. They're not. Extension methods are normal methods, with just a tiny set of restrictions. All the unrestricted stuff is a normal part of it, and something we've definitely seen people do since these have been around 20 years. Telling people for 20 years they could do that things and now taking it away just because you don't like them is not ok. Takebacks can be ok. But generally only when they are paired with some fantastic things to make that ok. Right now, the type approach is lacking that. It seems strictly less capable, and it puts people in the unpalatable position of having to bifurcate their codebase for safety. The member approach lets people move forward with complete safety, supports all the scenarios from before, and adds support for the cases people are asking for. Why go with a proposal with all the drawbacks? |
Beta Was this translation helpful? Give feedback.
-
It shouldn't be allowed for methods, either, and existing extension methods like that should be refactored (perhaps with a built-in analyzer and code fixer) to static helper methods.
Why not do
It has to be known, though. For example, the
To clarify, regarding the type-based approach, I'm not arguing either way about the emit strategy. Regarding emit strategy, I'm just asking for clarity of intention - not because I need it for my personal gain, but in order to be able to design the feature correctly. |
Beta Was this translation helpful? Give feedback.
-
Which are entirely subjective. |
Beta Was this translation helpful? Give feedback.
-
To an extent in other situations, but in this case, we have significant effort going into new syntactic constructs, major emit decisions, consequences for future language development - it is very objective in this case. |
Beta Was this translation helpful? Give feedback.
-
The irony :-)
I don't find your approach simple or elegant. As a consumer it would be very complex and inelegant. I'd have to keep extensions around for safety. And the only way to try to migrate would be to hang up to examine IL and hope my test suite missed nothing. Including the language now redirecting my callers to different extensions. |
Beta Was this translation helpful? Give feedback.
-
No, that's still definitely subjective. As is weighing the value of supporting a safe migration strategy for the existing feature set compared to potential future directions which aren't even considered yet. Language design is compromise. It doesn't matter how simple and elegant a solution seems to be if people won't adopt it. |
Beta Was this translation helpful? Give feedback.
-
This applies to both directions. |
Beta Was this translation helpful? Give feedback.
-
I think it's telling that I keep asking for the compelling scenarios that are so worthwhile that it's ok to give up compat, and that still hasn't been answered. Now the argument has shifted to it being a good thing to just break what extensions can do because someone doesn't like the capabilities of existing extensions. That's a non starter for me. Extension methods are huge. They are broadly produced and consumed. We would only consider breaks for things we were completely sure were mistakes that we'd been living with for decades. And even THEN we'd be cautious. You're suggesting breaking semantics that have been a normal, fine, intentional, part of the design since the beginning. And, again, without compelling value to make up for that. |
Beta Was this translation helpful? Give feedback.
-
So does the type approach.
Same for both. Why is the type approach objectively better?
Same for both. Why is the type approach objectively better? I personally feel like it is worse for future language development as it's too dissimilar for how library authors actually want to model things. So we would be going down a path with a higher chance of future problems. |
Beta Was this translation helpful? Give feedback.
-
I'm not sure whether I'm deviating here, I haven't been following through all the discussions and notes so excuse my ignorance but was the following syntax (using PC) suggested before? probably was but putting it here just in case it wasn't. :) public extension StringsExtensions(this string self)
{
} as opposed to the following: public extension StringsExtensions for string
{
} |
Beta Was this translation helpful? Give feedback.
-
I posted something like that to the LDM notes discussion: See: #8521 (comment) |
Beta Was this translation helpful? Give feedback.
-
I'm building to that. For me to state that the proposal is not ideal, I need to explain either how to do things differently, or why things don't need to be done. Again, leaving emit aside for a moment, the "extending a nullable" scenario should not be supported, IMO. Which means one of the reasons for the "compromise" of "new extension members" goes away.
Huh? I'm not saying we should break anything. Old extension methods stay around (and will likely stay around for a very long time). I'm not proposing to deprecate them with warnings or anything like that. (At most, we can give an analyzer, or maybe just guidance - a "best practice" line item that doing stuff like that is an anti-pattern.) PS. Going back to emit strategy, I still haven't heard a definite answer to the question, particularly regarding |
Beta Was this translation helpful? Give feedback.
-
That's your opinion. It seems that the language team does not share this opinion with you. I certainly don't either. You're making the assumption that this is considered a mistake of the design of extension methods, one that needs to be carried over to whatever this new feature is only because of legacy reasons, and that is not a correct assessment.
It's up to the team to assess how each aspect of a feature impacts the cost. It's not a blanket -100. Supporting nullable receivers is cheaper than not as the emit strategies are known and it's less work to enforce. It's something the runtime has always supported out of the box and the C# does extra work to prevent it. Struct wrappers would have to do the same additional work. |
Beta Was this translation helpful? Give feedback.
-
Care to explain why? Why is it so necessary to be able to write
It's not cheaper, though, because in order to support them, all of the syntax work needs to be done as proposed in the OP. The original syntax proposal for extensions was a very close match to regular type syntax, which was its strength. Now we're saying, we'll keep that, PLUS add all of these compromise solutions because we absolutely MUST replace ALL old extension methods. It's this last part I'm not seeing a good reason to do. |
Beta Was this translation helpful? Give feedback.
-
Because I find it convenient? It allows me to absorb the onus of the null check on helper methods in one place rather than force every consumer of that method to do so?
The syntax is not relevant as to whether or not an extension will perform a null check on the receiver.
Also not necessarily true. Even if the language team decided to completely skip binary compatibility with existing extensions, that doesn't say anything about whether or not null receivers would be supported. I'd suggest that they would continue to be, not because of some concern over legacy migration, but because it's a genuinely useful feature. And frankly because it's cheaper and easier than forcing the compiler to have to emit the null check in every extension, always incurring that cost even in cases where |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Compromise design for extensions
This has been moved to https://github.com/dotnet/csharplang/blob/main/meetings/working-groups/extensions/compromise-design-for-extensions.md.
Beta Was this translation helpful? Give feedback.
All reactions