-
Notifications
You must be signed in to change notification settings - Fork 1.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
Accept legacy language features #20454
Accept legacy language features #20454
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nice compromise. It just silently ignores legacy cruft. Legacy choices are not advertised in help message. I might expect a warning. But it does not warn for repeated choice -language:postfixOps,postfixOps
, compare -feature -feature
, so a relaxed approach is fine. It might have been nice to unify legacyArgs
, where the legacy form is empty string, but that is a stretch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution hard-codes the supported language imports. That means everytime we add a new one we also have to update this part. It might be better to explore a different solution. Don't check anything when we build Settings, but when we first compute enabledLanguageFeaturesBySetting
in Feature
. There we can check each option to see whether it corresponds to an object in the language
object.
The tricky thing is how to do the validation only once per run. We could have a flag in SettingsState
that tells us whether language imports have been validated.
The other tricky thing is how to add help messages without duplicating all imports in Feature
, like we do now. I don't have an answer to that. Without an answer I am not sure this alternative approach will work out.
This PR just lists strings not to error on. They have no other representation. (The principled solution would introduce deprecated feature flags, with labored text that they are no-ops for backward convenience; but that seems overkill.) |
Since we want to display every available feature and check users' input at the very beginging, I don't think it is a better solution to encode them in the |
Fix #20419