-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[API Proposal]: Add ObsoleteAttribute.BinaryCompatOnly #65965
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsBackground and motivationThe .NET Runtime goes to great lengths to have binary compatibility between releases. That dramatically lowers the cost of version to version updates and provides a smooth update experience for customers. It also means that API decisions are forever, once shipped we are stuck with an API. This is true even as the language and ecosystem evolves and better patterns emerge that we'd like to promote. For example consider the CallerArgumentExpression feature in C#. Given this feature it's possible to have a better default experience for public static class Debug
{
public static void Assert(bool condition, [CallerArgumentExpression("condition")] string message = null);
} This though is a bit of a non-starter. Due to the rules of C# this method will never be bound to because it will always prefer This desire to remove one method from source and replace with a highly compatible alternative has come up several times over the last few releases:
The proposal is to provide a mechanism by which APIs can be marked as "for binary compatibility only". The member will not be considered for source compilation purposes by languages. This can be done by extending our existing Note that this is substantially different than This proposal removes the API at a much lower level. It asks languages to not even consider the member during compilation. For all intents and purposes the member should not be imported from metadata except for what is necessary to ensure a type definition is sound (for example that it implements all interface members). API Proposalpublic sealed class ObsoleteAttribute : Attribute
{
public ObsoleteAttribute(string? message, bool error, bool binaryCompatOnly)
{
Message = message;
IsError = error;
BinaryCompatOnly = binaryCompatOnly;
}
public bool BinaryCompatOnly { get; set; }
} API Usagepublic static class Debug
{
[Obsolete(BinaryCompatOnly = true)]
public static void Assert(bool condition);
public static void Assert(bool condition, [CallerArgumentExpression("condition")] string message = null);
} Alternative DesignsLeverage reference assembliesAn alternative design is to change the reference assembly generation tool such that these APIs are just not included. That would have roughly the same impact as this change: on upgrade only the new APIs would be available for compilation hence they would win out. The downside of this approach is that it's going to be less friendly to languages other than C#, F# and VB. As detailed in CallerIdentityAttribute proposal it's likely that the alternative source methods provided are going to rely on new features. Those features are less likely to be present in other languages and hence is more likely to result in compilation errors on upgrade. This is not the case with the proposal to add Alternate attributeIt's reasonable to see this as expanding RisksMethod Group SupportThe motivating cases around this tend to be about overload resolution. Essentially an existing API is always preferred due to C#, VB, F# overload resolution rules and that is preventing us from adding a new, better API. The // Compiles today, compiles tomorrow just to a diff overload
Debug.Assert(SomeCondition); This will not fix method group conversions though as they are focused on the signature of the method // Compiles today, fails tomorrow
Action<string> action = Debug.Assert; This is likely a minority case compared to method invocation though and may be an acceptable trade off for forward progress in the platform.
|
I think you mean Because BinaryCompatOnly would affect overload resolution, it would need a C# specification change and should not affect C# language versions that have already been standardized. |
Correct that should've been
To be clear it impacts more than overload resolution. For all intents and purposes the symbol does not exist. The best way to visualize this is that the language ignores the member entirely when importing from metadata (details are likely to be different but practically this is how it will work). That means it impacts any feature which could resolve to that symbol. |
If metadata were compiled from public class A {
public virtual void M() {}
}
public class B : A {
[Obsolete(BinaryCompatOnly = true)]
public new void M() {} // not virtual
} and that were imported to public class C : B {
public override void M() {}
} then, would C.M skip B.M and override A.M, adding a row in the MethodImpl table? |
Marked as @jaredpar, is there any pressing need for this from the language for .NET 7/C# 11 or is it just part of the "general backlog"? |
Not on our side. This proposal is mostly a reaction to roadblocks I've seen runtime / aspnetcore hit recently trying to evolve to modern C# features. Hence there is no direct priority here and could be done in .NET 7 or 8. The one possible .NET 7 connection is CallerIdenityAttribute. I do not know if that is critical for .NET 7 or fine for a later version. It is hard to see how that could succeed the way we want without a proposal of this form. @jkotas, @agocke can likely speak best to the timeframe of that feature. |
This is why I was saying "visualize" vs. "this is how we are actually going to do it 😄 There are a number of details we'd need to work out on the language side. My expectation is that we'd import the member and keep it around for checks like duplicate members, duplicate types, exposing |
CallerIdentityAttribute contributes to our AOT and trimming friendliness .NET 7 theme. I would love to see it happen if possible. |
As an aside, I think the mechanics in this proposal are very similar to the spec language in ECMA around modreqs. If you do not understand a modreq as a compiler, you're supposed to "drop" the member on import. It sounds like |
Yes it's conceptually similar to a
|
Small thing: should the name be the full |
I'm fine with whatever name API review thinks is appropriate here. |
@jaredpar, the Debug.Assert example is a good one. You mention this would help in a few other cases that have come up with the core libraries... got additional examples of things we'd want to update at the same time? |
Examples that would be relevant if we went forward with CallerIdentityAttribute Generally applicable scenarios:
I don't know of any candidates in core libraries that meet these requirements today off hand though. |
We should probably sit down and list all the places where this would be interesting to use and see what the alternatives would be. For instance, for But we love the idea :-) |
FWIW this isn't a change, we already have the ability to suppress API from the reference assembly and do in some places today. I do like this proposal though, because it allows folks (presumably) to suppress this and presumably get the old overload resolution and temporarily unblock themselves, as they can with Obsolete. |
A big thing worth noting is that this applies far beyond just dotnet/runtime and the standard experience for ref assemblies doesn't allow members to be trivially stripped out and/or ignored. |
If this is something that the language team wants to commit to driving, we can move it back to 8. |
Background and motivation
The .NET Runtime goes to great lengths to have binary compatibility between releases. That dramatically lowers the cost of version to version updates and provides a smooth update experience for customers. It also means that API decisions are forever, once shipped we are stuck with an API.
This is true even as the language and ecosystem evolves and better patterns emerge that we'd like to promote. For example consider the CallerArgumentExpression feature in C#. Given this feature it's possible to have a better default experience for
Debug.Assert
by defining the method as such:This though is a bit of a non-starter. Due to the rules of C# this method will never be bound to because it will always prefer
Debug.Assert(string)
. Changing the overload rules that make this determination is a bit of a non-starter because it would result in sweeping changes across the ecosystem. Where as the desired impact is much smaller: stop usingDebug.Assert(string)
for languages that supportCallerArgumentExpression
.This desire to remove one method from source and replace with a highly compatible alternative has come up several times over the last few releases:
CallerArgumentExpressionAttribute
The proposal is to provide a mechanism by which APIs can be marked as "for binary compatibility only". The member will not be considered for source compilation purposes by languages.
This can be done by extending our existing
ObsoleteAttribute
to have a new property:BinaryCompatOnly
.Note that this is substantially different than
ObsoleteAttribute.IsError
. That in no way changes what members a language will bind to. Instead it is only considered after a language has gone through member look up, overload resolution, etc ... At the moment the final decision on a member is made only then does the language look atObsoleteAttribute
and determine a diagnostic should be emitted.This proposal removes the API at a much lower level. It asks languages to not even consider the member during compilation. For all intents and purposes the member should not be imported from metadata except for what is necessary to ensure a type definition is sound (for example that it implements all interface members).
API Proposal
API Usage
Alternative Designs
Leverage reference assemblies
An alternative design is to change the reference assembly generation tool such that these APIs are just not included. That would have roughly the same impact as this change: on upgrade only the new APIs would be available for compilation hence they would win out.
The downside of this approach is that it's going to be less friendly to languages other than C#, F# and VB. As detailed in CallerIdentityAttribute proposal it's likely that the alternative source methods provided are going to rely on new features. Those features are less likely to be present in other languages and hence is more likely to result in compilation errors on upgrade.
This is not the case with the proposal to add
ObsoleteAttribute.BinaryCompateOnly
. Languages have to make a change to recognize that attribute. Languages can then weigh the cost of supporting the features most relied on byBinaryCompateOnly
and take them as a single feature in a release.Alternate attribute
It's reasonable to see this as expanding
ObsoleteAttribute
beyond it's intended scope and instead a new attribute should be considered likeBinaryCompateOnlyAttribute
Risks
Method Group Support
The motivating cases around this tend to be about overload resolution. Essentially an existing API is always preferred due to C#, VB, F# overload resolution rules and that is preventing us from adding a new, better API. The
[Obsolete(BinaryCompatOnly = true)]
in combination with a new overload, which is largely source compatible, will minimize upgrade experience pain to a large degree for method invocations.This will not fix method group conversions though as they are focused on the signature of the method
This is likely a minority case compared to method invocation though and may be an acceptable trade off for forward progress in the platform.
The text was updated successfully, but these errors were encountered: