-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Open issues: Breaking changes #7918
Comments
Is there a way to generate telemetry from the IDE/compiler when these warnings are detected? Maybe even with some analysis to determine if the case in question fits an expected (and possibly auto-fixable) pattern? I could see a mechanism like that being used to gauge impact but also to test the potential tooling ahead of time. I could imagine that for more complex cases the team might want to wait more than one cycle to actually pull the trigger, or maybe even back off due to unforeseen circumstances. I am super happy that the team is considering breaking discards. I've always felt that it was one of those awkward warts and that the attempt to retain backwards compatibility ultimately caused more friction than it avoided. |
What about lambdas? e.g.: var lambda = (int _) => Console.WriteLine(42); |
@SunnieShine
IMO, I think the parameter declaration should be legal, but attempting to reference it as a variable in the lambda body should not be: // fine
var lambda1 = (int _) => Console.WriteLine(42);
// error
var lambda2 = (int _) => Console.WriteLine(_); |
@SunnieShine:
Good catch on the lambdas! We currently allow: Func<int, string, string> lambda1 = (_, _) => "Hello";
var lambda2 = (int _, string _) => "Hello"; I.e. when there is more than one The difference would be that a single |
At yesterday's LDM I was reminded that there is one more kind of potential break with class C
{
public int P
{
get { var field = "Field".Length; return field; }
}
} This would become an error in C# 13 because you are not allowed to redeclare an existing local or parameter. Unlike the other breaks discussed above, I believe this would be a problem even for some of the "non-breaking" alternative designs we've discussed in the past. It's simply a kind of break that we hadn't thought of. I see a couple of solutions:
Thoughts? |
There is also a parameter case, a lambda parameter or a local function parameter. |
Can C# be changed to consider public int Foo {
get;
set {
int @value = 123;
int @field = 456;
field = @value + @field;
}
} |
There would be subtleties and complexities with lambdas/local functions. But it's worth exploring. |
It feels like it may be a smooth transition from "special variable name" to "contextual keyword", most IDEs already colorize |
I dont think they are a problem, unless I misunderstand your point. Parameters are allowed to shadow existing locals: class C
{
public int P
{
set
{
Action<int> a = value => { }; // No error
void F(int value) { }; // No error
var value = 0; // CS0136
}
}
} |
I am not saying that there is a problem, I simply do not know details of the current design to judge about that. I am saying that a |
That's definitely worth considering. A lot of people are surprised when they discover that This would allow code like this: class C
{
public int P
{
get
{
var @field = ""; // Allowed - field is a contextual keyword
return @field.Length + field; // both @field and field available
}
}
} People who have a local called In fact, for all the breaking cases outlined above, this would allow the default fix to similarly replace every breaking occurrence of We'd need to carefully evaluate whether making |
Note: from a parsing perspective, we could make it a literal keyword, parsing accessor bodies in a special state (just like we do for async/await parsing). We would just need to be careful in incremental scenarios to ensrue that such state was encoded into the node. Unfortunately, we literally only have 8 bits that we can use to store this data for, and we're already full-up on bits :'( |
Riffing further on this, the context within which |
Well shucks, there goes that feature design then! ;-) |
Looking at this more, I don't think we can achieve all of these three goals at the same time:
Here's the counterexample: class C
{
private int value;
private int field;
public int P
{
set
{
WriteLine(@value); // References the implicit parameter, not the field
WriteLine(@field); // We would want this to reference the field
}
}
} I think we could choose to budge on either of the three goals above:
|
That'd be my preference, I can't imagine it's remotely common to find developers using |
I see |
What about |
Yes and also it looks like the cleanest possible state going forward; if we were to break, it should be for a good reason. |
Clearly "good" is in the eye of the beholder. Improving the user experience through improved syntax consistency and embracing modern language idioms is clearly a good reason to break in the eyes of many. What languages like rust - with its clever breaking changes support mechanism - show us is that blocking breaking changes is an impediment to progress. What rust does - with the benefit of massive hindsight - is make breaking changes manageable. And that's what C# needs too: a robust mechanism for managing breaking changes to make those breaking changes desirable rather than to be feared. |
Rust is indeed a great example with "editions". If language semantics change in a new edition, a simple Essentially, if My $0.02: If breaking changes through language versions were to be implemented, I'd request that the team provide something similar to |
This is something we need to take our time with. Let me share my opinions: Make
These are pretty rare cases where these would break code, and most are a single @, rename, or text replace away. Cases where breaking changes here might include:
I won't say none of us name our variables not-so-great things, but really, I almost see this as a way to enforce better naming. Edit: Added code blocks |
Based on the discussion above around making Thanks everyone for all the insights here! ❤️ |
Regarding field access, it seems that there must be a non-breaking way to address this, perhaps: public class Professor
{
private string field;
public string Field { get => Field.field; set => Field.field = value.Trim(); }
...
public override string ToString() => $"Professor of {field}";
} The name Or one could even use this: public class Professor
{
private string field;
public string Field { get => Field:Field; set => Field:Field = value.Trim(); }
...
public override string ToString() => $"Professor of {field}";
} Here the implicit backing field is now always the same name as the property itself, not the special name |
There are plenty of non-breaking ways to address this, but they all have warts that create inconsistencies with the already existing identifier |
This comment was marked as spam.
This comment was marked as spam.
what about instead of calling the new feature "field", to call it "backing" ? While this doesn't address the root cause of the breaking change, i think this variable name is less used in the wild. |
That has exactly the same problems, but worse in practically every other way |
Breaking changes mitigation - open issues
In order to embrace a breaking-changes approach in C#, we need to a) decide on general principles and procedures, and b) decide how to apply these concretely to the "Field access in auto-properties" feature for shipping in C# 13.
In the following are a number of concrete proposals, along with discussion, examples and alternatives. The intent is to drive to complete design decisions on both of those questions.
Running examples
We'll examine the impact of the decision points in the context of the new "field access" feature, as well as some existing features that would have been - and may still be - candidates for breaking changes:
var
and discards.Field access
With the field access in auto-properties feature, property accessor bodies will now have access to an implicitly declared parameter called
field
representing the generated backing field of an auto-property. This is a breaking change, because current property accessors may well usefield
to refer to something declared outside of the property:After the language change, the
field
inside of theget
andset
accessors would now refer to a generated backing field, whereas thefield
in theToString
implementation refers to the declared fieldfield
.var
When
var
was introduced to C# it was potentially breaking, because there might be declared types calledvar
:To avoid this breaking change, however far-fetched, we made a stopgap rule that
var
would only mean "infer my type" if no type calledvar
was in scope.(People have since abused this rule to prevent others on their team from using the
var
feature, by declaring a useless type of that name in the project!)This is an example of "spooky action at a distance": A declaration far away changes the meaning of code from otherwise locally determined semantics.
Discards
Discards are simple names
_
(underscore) and local variable declarationsint _
andvar _
where the variable introduced is discarded, and cannot be referenced from code again.Declaration and use of
_
as an ordinary local variable was already allowed, so in order to avoid a breaking change we limited discards in two ways:These limitations mean that "old fashioned" local variable declaration syntax is poison to the use of discards, and make it confusing to understand when a given
_
is a discard and when it isn't:Here all the
_
s that are not commented out are discards. However, if you uncomment the old-fashioned local variable declarations at the top, all kinds of things break:Fixing this would be a breaking change. There's an interesting range of possible fixes, from the most to the least breaking. If we were starting from scratch we would probably make
_
a keyword and not an identifier. That would be the cleanest design, but at this point would also lead to the most breaks in existing code, given how many different things can have names!At the other end of the spectrum, the user experience would probably still improve significantly if:
_
were discards, and_
as a simple name were discards.This should be enough to alleviate the current confusion and would be a much smaller break. As a bonus, today
@_
is only allowed when_
is a variable, not when it's a discard. So existing code could continue to use_
as a local variable name by prefixing it with@
.Deciding on a breaking design
Breaking changes are disruptive! It's useful to have a set of criteria for the C# team to adhere to when considering a significant breaking change, to ensure that it's worthwhile to do and comes with sufficient mitigation. These are the proposed criteria (first suggested here):
Verify: Do we agree that this is a reasonable set of criteria?
Below we explore the use of these criteria on each of the example breaking features.
Deciding on
field
Here is how we would justify the
field
breaking change by the criteria:field
is used inside property accessor bodies. It will certainly occur ("field" is a common noun with several uses), but not all over the place, and in most code-bases not at all.A
, change it toA.field
.this.field
.field
that is initialized from the parameter, then change the simple name tothis.field
.Verify: Do we agree that this justifies the breaking design for field access? Do we agree that the proposed default fix is suitable?
Deciding on
var
The
var
breaking change would be justified as follows:var
as a type name today. The accommodation for it only leads to user confusion, as well as to implementation complexity and degraded experiences in compiler and tools.var
as a type name to thwart usage. Nowadays that is better achieved with code styles.var
that would no longer refer to a type in a future release.var
with@var
.Deciding on
_
The
_
breaking changes would be justified as follows:_
is a discard is severely undermining its use._
as a local variable today are remnants of pre-discard code, to signify that a local variable should be considered a discard! Those cases would actively benefit from becoming "real" discards, and the remaining cases are rare._
and point out that it will be a discard in the future. We could avoid most false positives by ignoring variables that aren't subsequently read from._
to@_
.Mitigating a breaking change
Let's say a breaking change is introduced in C# N. The impact of the break on existing code is mitigated as follows: The compiler for C# N, when compiling for language version N-1 or lower, will produce a warning on code that will eventually break (change its meaning) in C# N. The warning will be the diagnostic message identified in criterion 3 above, and will suggest the fix identified in criterion 4.
IDEs, upgrading tools etc should offer the default fix to the impacted code locations, and would most likely offer to "fix all occurrences" across project and solution, so the user can fortify their code against a language version upgrade with a single gesture. In addition, there might be other reasonable fixes to offer (e.g. rename a declaration of something called
field
,var
or_
).Open question: It's possible for someone to still end up going to the new language version without having fortified their code against the breaking change. If they've already started using new language features, it's not necessarily feasible for them to go back down a language version to get the warnings. Should the compiler also provide off-by-default warnings for the post-upgrade situation, allowing folks to locate code that may have already changed behavior? Presumably this would be turned on to "audit" code, and would eventually be turned off again once the developer is satisfied that all occurrences are correct according to the new semantics.
Mitigating a breaking change warning
The new warnings on existing code are themselves breaking behavior. They are tied to installing a new compiler, and hence usually to installing a new version of the .NET SDK. It turns out that there are other ways in which the SDK would like to introduce new diagnostics for existing projects. We're therefore doing work to harmonize our approaches to this.
New warnings may be disruptive to the user. First, we would reduce the number of times this could happen to once a year, allowing such new diagnostics only on major-version upgrades of the SDK.
Second, the compiler would respect a flag to turn off not just a specific breaking change warning (which you can always do) but also all breaking change warnings. This would also be wired in to an SDK-wide flag to silence all such warnings from across the stack.
For some scenarios it would be meaningful to turn all breaking change warnings off permanently: Legacy projects that would never use a new language version could safely ignore them. Build servers could safely assume that the warnings are caught and dealt with elsewhere.
In other scenarios the warning may just come at an inconvenient time, and you want to postpone dealing with it. Perhaps IDEs and other tools can offer a "silence for 30 days" option or something like that.
Special language versions
In most cases the language version is explicitly specified to the compiler, usually by being implied by the target framework (TFM). Thus, when you get a new compiler, it will initially be configured to compile to an older language version than the newest one it supports. This is where the user normally has a chance of acting on the breaking change warnings!
Latest
However, there is also currently a
latest
language version, which silently picks the newest language version the compiler is capable of. This is unfortunate, because there is never a time when the language version is older than the compiler, and consequently the user won't get any breaking-change warnings!The proposal is to "retire"
latest
, in the sense that the use of it will start yielding a warning. If the warning is ignored or suppressed, then people are knowingly signing up to deal with any breaking changes by other means.Alternative: We could "park" the meaning of
latest
at C# 12, either with or without a warning to say thatlatest
no longer works as you'd expect. This way, users would still get breaking change warnings, and would quickly be discouraged from usinglatest
.Preview
The
preview
language version is likelatest
, but additionally enables language features that are not yet shipped, for the purposes of exploring and providing feedback. These features may end up taking a different shape, or not ship at all, so any user ofpreview
is already putting themself at risk of breaking changes down the line. In fact, if they usedpreview
with the previous version as well, they likely already used the breaking feature with its new semantics before it was even released!It therefore seems reasonable that users of
preview
get no warnings by default.Conclusion
With a good set of decisions here, we will be set up to handle not just the "field access" breaking change, but also future breaking designs. That said, this is a new avenue for C#, and there is a good chance that there's something we overlooked or misjudged. By shipping a single breaking feature in C# 13, we have an opportunity to learn and adjust, and to improve the process and experience for next time.
The text was updated successfully, but these errors were encountered: