Skip to content
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

F#8 #15793

Merged
merged 20 commits into from
Aug 18, 2023
Merged

F#8 #15793

merged 20 commits into from
Aug 18, 2023

Conversation

vzarytovskii
Copy link
Member

@vzarytovskii vzarytovskii commented Aug 14, 2023

We want version 8 to be default in rc1, we'll still get nullables into 8, and likely records interop.

@vzarytovskii vzarytovskii requested a review from a team as a code owner August 14, 2023 09:52
@abonie
Copy link
Member

abonie commented Aug 14, 2023

I am wondering, shouldn't these unit tests stick to withLangVersionPreview? I would assume these tests should also work with preview going forward, if we change the tests we might miss some regression, no? And if there is an explicit decision that something should work differently under preview, only then would we change the test from withLangVersionPreview to withLangVersion80

@vzarytovskii
Copy link
Member Author

vzarytovskii commented Aug 14, 2023

I am wondering, shouldn't these unit tests stick to withLangVersionPreview?

No, we want to make sure they work with version the feature is in.

I would assume these tests should also work with preview going forward, if we change the tests we might miss some regression, no? And if there is an explicit decision that something should work differently under preview, only then would we change the test from withLangVersionPreview to withLangVersion80

Version check does pretty much >=, so it will work with older versions. The whole idea is to check the version which feature is enabled in.

In other words, we're not really interested if the feature X works in preview, we're only interested if feature work in version Y, which it's a part of.

@KevinRansom
Copy link
Member

No, the tests need updating to the published language version

@vzarytovskii
Copy link
Member Author

vzarytovskii commented Aug 16, 2023

@T-Gro with langversion 8 by default, some tests are failing with:

failwith "getTypeRefsInType: modified"

Like innerpolly test, if you change it to run something like

[<Fact>]
let ``innerpoly-FSC_DEBUG`` () = 
    singleTestBuildAndRunAux "core/innerpoly" [ "--multiemit-" ] FSC_DEBUG

and run it, you will get the issue above

dotnet test .\tests\fsharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Release /p:BUILDING_USING_DOTNET=false --filter Miscellaneous.FsharpSuiteMigrated_CoreTests.innerpoly --no-restore -f net8.0

Without running multiemit, the fail is a bit mor cryptic:

 All errors:
[{ Error = Error 193
   Range = { StartLine = 1
             StartColumn = 0
             EndLine = 1
             EndColumn = 0 }
   NativeRange = (1,0--1,0)
   Message =
    "internal error: Unable to load one or more of the requested types.
Method 'Specialize' on type 'func@310-9' from assembly 'FSI-ASSEMBLY, Version=0.0.0.32767, Culture=neutral, PublicKeyToken=null' tried to implicitly override a method with weaker type parameter constraints."
   SubCategory = "interactive" }]

I will probably move unmanaged feature back to preview, until we fix it.

Now when we emit it, probably need to handle it (strip it?) in ilreflect.

@vzarytovskii
Copy link
Member Author

vzarytovskii commented Aug 16, 2023

A bunch of autocomplete tests are failing due to Obsolete handling (https://dev.azure.com/dnceng-public/public/_build/results?buildId=375838&view=ms.vss-test-web.build-test-results-tab&runId=7996012&resultId=100772&paneView=debug)

@edgarfgp if I remember correctly, you were working on something related, do you think it's by design?

Edit: I would say so, since in F#8 we're treating ObsoleteAttribute on type the same way we do on methods, and we hide methods with obsolete attributes. I will fix the test.

@edgarfgp
Copy link
Contributor

A bunch of autocomplete tests are failing due to Obsolete handling (https://dev.azure.com/dnceng-public/public/_build/results?buildId=375838&view=ms.vss-test-web.build-test-results-tab&runId=7996012&resultId=100772&paneView=debug)

@edgarfgp if I remember correctly, you were working on something related, do you think it's by design?

Edit: I would say so, since in F#8 we're treating ObsoleteAttribute on type the same way we do on methods, and we hide methods with obsolete attributes. I will fix the test.

Yes, my changes were about making sure the Obsolete attribute is raised at the `type, modules, methods, etc. So I imagine some auto-complete tests will be failing now as the obsolete will be correctly applied

@vzarytovskii
Copy link
Member Author

Just a note, I had to disable couple of tests in the old vs language service, which is not in use.
For some reason tests weren't working there, but similar ones were fine in FCS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants