Conversation
❗ Release notes required
|
auduchinok
left a comment
There was a problem hiding this comment.
Thanks for this @kerams!
What do you think about keeping the parser rules and just always producing the errors? We could easily remove them in future if wanted to repurpose these constructs in the language. For now keeping them would allow to parse older code which could be nice as long as it doesn't need maintenance in the parser.
That's been the case since ML compatibility was deprecated. The only way to get rid of it was the combination of the 2 flags. We could keep the parser rules, but one of the benefits of doing this was a slight parser/lexer simplication, and I'm not sure what would be the practical benefit of preserving them. |
I think keeping the simple rules that don't require any additional support (like the type arguments in parens) would be beneficial. I see it as a sort-of error recovery rules. |
|
Yes, long term code simplification should be the motivator here, since the rules were there but obsolete for many years. (and it is always possible to e.g. copy existing parser rules+code and build a dedicated "ML-style-syntax-tree nuget" with it.) |
This comment was marked as outdated.
This comment was marked as outdated.
Is it because of the changes this PR did? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Added. I just throw on <8, there's no SDK <-> language version matrix. An exercise for someone else down the line;) I know it would be more accurate to say that features in version %s have become part of core F# and are always enabled, but that specifically doesn't apply to ML compatibility. Feel free to change it however you see fit. |
|
The wording is good, thanks for adding that 👍 . I am also thinking about building an unsupported (= not officially shipped) .vsix right before this change and uploading it to people in case they need to maintain such an unsupported codebase. But I can do that JIT when/if the request comes at all. (reasoning is that pre-change tooling will be better in migrating the codebase, since it will give more accurate warnings) |
Changing in order to align with dotnet/fsharp#19143
T-Gro
left a comment
There was a problem hiding this comment.
This contribution simplifies the compiler codebase, especially the lexer and parser.
Big thank you to @kerams for starting the activity of stabilizing the "core of F# language" by reducing the number of supported versions, and taking the largest language feature by far!
I added a few minor comments only.
Description
Implements fsharp/fslang-suggestions#1407
asr,land,lor,lsl,lsr,lxor(modcontinues to be reserved)orand&operators in FSharp.Core for binary compatibility^string concatenation operator in FSharp.Core has been marked as obsolete (withIsError = true).mland.mlisource files#lightand#indentdirectives (they are now a no-op, combined with"off"they give an error)--light,--indentation-syntax,--no-indendation-syntax,--ml-keywordsand--mlcompatibilitycompiler/fsi flags#lightfrom source code. There are hundreds of these files, but they can be safely skipped during reviewChecklist
Test cases added
Performance benchmarks added in case of performance changes
Release notes entry updated: