dotnet / vblang

The home for design of the Visual Basic .NET programming language and runtime library.
289 stars 65 forks source link

[Discussion] Loud (Non-Silent) Breaking Changes, e.g. New Keywords #312

Open AnthonyDGreen opened 6 years ago

AnthonyDGreen commented 6 years ago

I'm just posting this for discussion. But I've been giving a lot of thought the how we handled the breaking change of making NameOf a reserved keyword in VB14. It's certainly not the first version of VB to introduce new reserved keywords:

But, in VB2012 we decided to make Await contextual because of heightened concerns about an in-place update to the compilers changing the meaning of code, potentially failing to compile, on a live running dynamically compiled ASP.NET web app.

Oddly enough, in that release both VB and C# took a silent* break to the way For Each variables are captured by lambda and query expressions. In VB it wasn't entirely silent since earlier versions of the compiler always warned that the previous behavior was busted and likely to change in a future release.

In VB2015 we considered introducing the approach C# uses for var and dynamic to VB for the NameOf feature. For those unfamiliar, in C# var and dynamic only take on their special meaning in C# if not types exist with those names already. Likewise, nameof(expression) only works to get the name of the provided symbol if no method named nameof exists. We decided against it based on a number of things including MVP feedback (*cough* Kathleen *cough*), VBs prior history of adding new keywords, and the downstream cost of such a design to tools authors like the IDE team and analyzer writers. The amount of life wasted for the edge case of var binding to a type is ... considerable.

The problem with making the keyword semantically contextual, like C#, is that in a context where that symbol has been defined new features become unusable. We've seen projects where someone has defined a type named var in the global namespace to block all use of type inference in that project (or any project that references it). If a method named NameOf is in scope, in some cases there's no way to get it out of scope and the NameOf feature would be unusable in VB in those places.

I don't regret the choice to go all-in on the keyword but I have been playing around with alternative approaches for the future. One of which is to turn the C# approach on its head. Instead of looking for an existing method named NameOf and only exposing new functionality if that fails, we could prioritize the new behavior but report a warning in the very rare case that the meaning of code changes. By reporting a warning we give the IDE a change to provide a quick-fix ("Fix All In Solution") which can update all occurrences in a solution at once to make the code back-compatible (by escaping the keyword). This avoids the silent breaking change problem and ensures that code which is being round-tripped between two versions of Visual Studio will mean the same thing in both versions, while not forever encumbering future code and future generations of users with the trivia legacy that "var wasn't always a keyword, you know".

This approach could be used in some other cases where the community has debated adding new keywords like Null and .Me (#31) or even to fix some places where the compiler is just broken. e.g. type inference doesn't work with Static locals. No language reason for that (I suspect it's an implementation detail/constraint). Instead of having this wort on the language forever:

' Type of l is actually 'Object' because static locals
' don't care about your Option Infer settings.
Static l = New C
l.Foo() 

We could fix the bug and on upgrade users would get a clean message in the error list:

"Semantic change: Due to a bug in previous versions the type of 'l' was 'Object'. Due to late-binding you might not have noticed this. To change this code to match its old behavior in both current and previous versions of VB, hit Ctrl+." and the fix turns it into: Static l As Object = New C. After fixing the code you could just silence the warning for the project and start writing code "the new way".

I'm not saying this is a perfect solution or that it should be used all the time. Just that it could be a good intermediate lever between Option statements and contextual keywords that strikes a good balance between discover-ability and providing the cleanest semantics for first-time users out of the box.

What do you guys think?

Would you be ok moving to a new version of VB if you were told up-front if any code changed meaning with a one-click fix to restore the old behavior? Are there any proposed language changes you could see taking advantage of such a mechanism? Where would you draw the line?

bandleader commented 6 years ago

@AnthonyDGreen I have been playing around with alternative approaches for the future. One of which is to turn the C# approach on its head. Instead of looking for an existing method named NameOf and only exposing new functionality if that fails, we could prioritize the new behavior but report a warning in the very rare case that the meaning of code changes.

  1. If the concern was "an in-place update to the compilers changing the meaning of code, potentially failing to compile, on a live running dynamically compiled ASP.NET web app," ...then your solution (which only raises a warning, which would never be seen) still leaves that a problem, no?
  2. Also, what issue do you have with the C# way of "looking for an existing method named NameOf and only exposing new functionality if that fails?"
paul1956 commented 6 years ago

When NameOf came out I moved to it quickly because of a CodeFix but then found out I had to use #IF on every use to get the code to work with older compiler that I was still using. The fact that I got an error/warning made it easy to find and write a code fix (it was my first). I like @bandleader approach as long as we have an easy way to specify which version of compiler I require.

KathleenDollard commented 6 years ago

@bandleader - what about for a major version change? Would only having a warning work across a major version change?

@paul1956 What scenarios result in you using an older version of the compiler? An older version of the runtime I understand. Have you seen breaks across compilers?

Everyone: Do you resolve your warnings? Do you know you can filter VS by a specific warning by number?

Doing anything on a warning requires that people actually care about warnings.

paul1956 commented 6 years ago

@KathleenDollard I had production code using production Visual Studio and was working on next release using a preview of next release of Visual Studio and had already used NameOf. I also had shared source code between different solutions using links. I have stopped using links because it caused too make issues when I shared source code with others. I ship with absolutely no warning but don't use warning as error (this was also the policy at the past two companies I worked for) and, no suggestions where possible. There are some suggestions I am unable to remove due to them being incorrect and those I report as bugs, and some are very confusing and the recommended fixes don't work for VB, this mostly seem to happen where C# and VB share common error messages and the fixes only apply to C#.

No, I did not know I can filter warnings, but wasn't looking to do it. If you want people to care about warnings change the default of Treat Warnings as Errors with every(major) compiler release and force developers to set it back after at least seeing the warning once. Even on ASP.NET I don't know people that deploy a new compiler without at least making sure the program compiles.

reduckted commented 6 years ago

@KathleenDollard Everyone: Do you resolve your warnings?

Yes. I have TreatWarningsAsErrors turned on in the Release configuration so that any warnings will break the build.

AnthonyDGreen commented 6 years ago

@bandleader

1st question: It's not an issue because the compiler isn't updated in-place anymore. So the .NET Framework 3.5 installs over .NET 2.0; it replaces the existing compiler. 4.0 installs side-by-side as I recall and later compilers just aren't shipped in the .NET Framework anymore so the scenario of a Windows Update breaking a dynamically compiled ASP.NET website running in production can't happen anymore.

2nd question: It's without precedent; we've never done it like that before. It's pretty awful for tooling authors including the IDE team and all analyzer writers. Essentially, every tool for the rest of time has to look at all invocations and check for the case where they're NOT NameOf. The amount of life wasted on the "var might not be var" case is pretty ridiculous. That's one reason why the cost of VB NameOf was not insignificantly less than C# nameof because not only does the compiler team have to write and test for the edge case the IDE team also has to write and test for both cases and then any downstream analyzer authors who want to detect or recommend the feature also has to write around it and then it just adds this neat piece of trivia you have to explain to everybody until the end of time in docs and what not. So it's a lot of burden on a lot of people with not clear and overwhelming benefit that was never felt valuable before after introducing MANY new keywords to the language over several releases that would be happening for the first time ever in VB just because C# has done it that way in the past. That wasn't enough reason.