Implement Langversion switch for implicit yields

[KevinRansom]

1. Adds back the 4.6 versions of the tests.
2. Adds back some validations that were removed for the 4.7 feature change
3. Places behavior change under Features.ImplicitYield flag
4. Updates fsharpqa suite slightly, to enable some versioned neg testing.

[TIHan]

A quick overview of the change, it looks fine. However, I am a little worried of the complexity; I haven't determined if that complexity is avoidable. It depends on what the change for implicit yields did. Will look at that tomorrow.

Tests are great. Though I wish we could start doing them in our unit tests projects.

[KevinRansom]

@TIHan, amusingly, I was surprised how small the actual change was. I was wondering why I spent so much time on it :-)

[TIHan]

Only reason why I say it's complex is because there are a lot of checks to see if we support the implicit yield feature or not for such a small change. But, as I said it may be unavoidable.

[KevinRansom]

😊

[cartermp]

Looks fine overall, I agree with @TIHan that it's kind of lame that the flag has to be checked like that, but I suppose that's a consequence of how checking these kinds of expressions is implemented and not really a consequence of LangVersion itself.

[cartermp]

Nit: could this be moved up so that it's below YieldOrReturn?

This was a little hard for me to read since I had was mistakenly thinking that the previous cases would also evaluate to false before I realized there was a guard here.

[dsyme]

I think the aim is to re-introduce the exact same code that was present for prior versions of F#, so we should keep it exactly as it was

[dsyme]

@KevinRansom Can you move this back to the matching location so the diff is minimized?

[KevinRansom]

@ dsyme, this is the original location … You moved it, once it was no longer necessary in |SimpleSemicolonSequence|. However, it needs to be in there to verify YieldFree (ness) for IfThenElse.

[dsyme]

@KevinRansom Do you think it might be feasible to redo this with completely separate paths for the old version and the new version? It is really hard to review with the two merged into together. I mean, literally two copies, one SimpleSemicolonSequence and one SimpleSemicolonSequence_OLD etc.

Then we can confidently code review each - the first by comparison with the PR (little or no change) and the spec, the second by exact comparison with the prior code.

[KevinRansom]

@dsyme, I think doubling up on YieldFree with impls for 46 and 47+ will take care of the bulk of the complexity.

[TIHan]

Looks good to me. Thanks for getting rid of a number of the SupportsFeature checks.

[dsyme]

Thank you @KevinRansom, that's soooo much cleaner. In general it feels like we should aim to have about 1-3 "supports feature" checks in the code for each language feature.

[KevinRansom]

@dsyme, from now on feature implementers, have the joy of doing this :-)

[dsyme]

@dsyme, from now on feature implementers, have the joy of doing this :-)

What if we ask nicely? You're so good at it! :)

[realvictorprm]

Yay :)

[smoothdeveloper]

What if we ask nicely? You're so good at it! :)

@dsyme, you also need to keep @KevinRansom out of any discussion involving "file ordering" in the compiler for at least 2 months 🙂

What if we ask nicely @KevinRansom to edit a markdown note about using this language feature check so that new commers can follow the instructions?