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: language support for Obsolete #11583

Closed
ljw1004 opened this issue May 26, 2016 · 4 comments
Closed

Proposal: language support for Obsolete #11583

ljw1004 opened this issue May 26, 2016 · 4 comments

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented May 26, 2016

Background

In C#6, we can basically never remove nor obsolete an old method:

// SCENARIO 1:
int f(string s);     // my library v1 has this method
int f(string s, bool b = false)    // I want library v2 to use this method
// I can't remove the first overload because of binary-compat
// I can't obsolete the first overload because overload resolution of f("a") prefers it

// SCENARIO 2:
float g();     // my library v1 has this method
double g();  // I want my library v2 to use this method
// I can't remove the first overload because of binary-compat
// I can't even declare the second overload because in C# you can't differ only in return type

I propose we add language support for Obsolete:

// SCENARIO 1:
obsolete("error",true) int f(string s);
int f(string s, bool b = false)
// The first candidate remains present (which solves binary compat)
// Overload resolution will always prefer the second candidate

// SCENARIO 2:
obsolete("disliked",false) float g();
double g(); 
// The first candidate remains present (which solves binary-compat)
// Overload resolution will prefer the second candidate

Proposal

Syntax

Add a new method modifier obsolete

obsolete ::= "obsolete" "(" string_literal "," boolean_literal ")"

class_modifier ::= ... | obsolete
constant_modifier ::= ... | obsolete
field_modifier ::= ... | obsolete
method_modifier ::= ... | obsolete
property_modifier ::= ... | obsolete
accessor_modifier ::= ... | obsolete
event_modifier ::= ... | obsolete
indexer_modifier ::= ... | obsolete
constructor_modifier ::= ... | obsolete
struct_modifier ::= ... | obsolete
interface_modifier ::= ... | obsolete
enum_modifier ::= ... | obsolete
delegate_modifier ::= ... | obsolete
fixed_sized_buffer_modifier ::= ... | obsolete

Semantics

  1. Define less obsolete: a method without the modifier is less obsolete than obsolete(...,false) which is less obsolete than obsolete(...,true). I'm using false=warning and true=error here, just the same as the [Obsolete] attribute.
  2. It is an error to have an obsolete modifier on something that also has an [Obsolete] attribute
  3. obsolete(s,b) implies [Obsolete(s,b)], along with all the associated warnings+errors that this implies
  4. The obsolete modifier is not considered part of a method's signature.
  5. The rules for class members currently say "the signature of a method must differ from the signatures of all other methods declared in the same class". This will be amended to say that two methods in a class are permitted to have the same signature so long as (1) they differ in return type, (2) either one of the methods is less obsolete, or the class has a third method with the same signature which is less obsolete
  6. The rules for better function member currently have rules to say whether Mp is a better function member than Mq. This will be amended with an even more important preliminary check: a less obsolete function member Mp is always better than a more obsolete member Mq.

Compilation

Confusingly, we have to prevent obsolete from itself being a back-compat break. Here's the scenario we have to worry about: (1) A user of C#6 references a library which uses obsolete methods, but C#6 is unaware of them, and so picks a more obsolete overload candidate; (2) the user of C#6 upgrades to a version of C# with this proposal, which now changes the overload candidate.

We can't make the obsolete modifier be a modreq because the CLR treats modreqs as part of a method's signature, and so adding the modifier would break binary compat. This would defeat the entire point of the proposal.

Therefore: we will emit obsolete(s,b) as [Obsolete(s,b), ObsoleteModifier]. C#7 should recognize this [ObsoleteModifier] attribute on a method when it meta-imports, and mark the method and all its overloads as bad.

Later on, once we've seen that usage of C#6 has declined sufficiently, then we introduce the feature proper into C#8 or C#9, safe in the knowledge that it won't be a severe back-compat break.

@HaloFour
Copy link

This sounds like it would cause nightmares in cross language and cross version support. The existing compilers don't play well with methods that are overloaded by return type. As long as C# can target earlier frameworks there will always be the likelihood that a C# 9.0 binary will need to be consumed by a project written and maintained in Visual Studio 2005, likely by one of those large influential customers who would be very pissed off if any behavior changed on them.

@dsaf
Copy link

dsaf commented May 27, 2016

@HaloFour

...need to be consumed by a project written and maintained in Visual Studio 2005, likely by one of those large influential customers who would be very pissed off if any behavior changed on them...

In practice causing enterprise harm is not really a big consideration for Microsoft to be honest:

https://medium.com/swlh/how-one-announcement-destroyed-the-net-ecosystem-on-windows-19fb2ad1aa39

https://news.ycombinator.com/item?id=11761437

@HaloFour
Copy link

@dsaf

How Microsoft treats new and isolated platforms isn't relevant to their general behavior of obsessively maintaining backward compatibility. Microsoft did not break how existing .NET apps worked or built on Windows 8 or Windows 10.

This proposal demonstrates they're relatively obsessive about ensuring forward compatibility between versions of the .NET framework and compilers to the extent that no new warnings can be added or existing members marked obsolete. In general I applaud Microsoft for their efforts here, but I do think that they take it a bit far. Other widely used enterprise languages do add new warnings or breaking changes. For example, Java 8 added a new warning about using _ as an identifier stating explicitly that doing so in a future release will likely be a compiler error.

@svick
Copy link
Contributor

svick commented May 27, 2016

The two scenarios don't sound convincing to me:

In scenario 1, you have to keep the old overload around for binary compatibility, so it has to keep working. Then what is the advantage to hide the overload when recompiling? The one I can see is that the user gets to see only the new overload, but that doesn't seem like a big advantage to me.

In scenario 2, you're changing the return type of a method, which is a source breaking change. So what's the point of avoiding binary breaking change? When the user recompiles, the code will (potentially) break anyway.


Another point I don't like is the syntax: obsolete("error", true) having different meaning than [Obsolete("error", true)] is confusing. And having special syntax for this doesn't seem worthwhile to me for such a niche feature.

The compiler already treats [Obsolete] in a special way, so why not just expand on that? Either by directly adding [ObsoleteModifier] (which was proposed as the compilation strategy) or by creating new ObsoleteAttribute overloads.

@gafter
Copy link
Member

gafter commented Mar 24, 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.


I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this.

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

5 participants