Merge release/dev16.2 to release/dev16.3

eng/Versions.props

@@ -23,14 +23,14 @@
     <FSharpCorePreviewPackageVersion>$(FSCorePackageVersion)-$(PreReleaseVersionLabel).*</FSharpCorePreviewPackageVersion>
   </PropertyGroup>
   <PropertyGroup>
-    <FSPackageMajorVersion>10.4</FSPackageMajorVersion>
-    <FSPackageVersion>$(FSPackageMajorVersion).3</FSPackageVersion>
+    <FSPackageMajorVersion>10.5</FSPackageMajorVersion>
+    <FSPackageVersion>$(FSPackageMajorVersion).0</FSPackageVersion>
     <FSProductVersionPrefix>$(FSPackageVersion)</FSProductVersionPrefix>
     <FSProductVersion>$(FSPackageVersion).0</FSProductVersion>
   </PropertyGroup>
   <PropertyGroup>
     <VSMajorVersion>16</VSMajorVersion>
-    <VSMinorVersion>1</VSMinorVersion>
+    <VSMinorVersion>2</VSMinorVersion>
     <VSGeneralVersion>$(VSMajorVersion).0</VSGeneralVersion>
     <VSAssemblyVersionPrefix>$(VSMajorVersion).$(VSMinorVersion).0</VSAssemblyVersionPrefix>
     <VSAssemblyVersion>$(VSAssemblyVersionPrefix).0</VSAssemblyVersion>

eng/targets/Settings.props

@@ -8,7 +8,8 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <IncludeVsLanguageServer>true</IncludeVsLanguageServer>
+    <!-- this should be false for branch `release/dev16.2`, true otherwise -->
+    <IncludeVsLanguageServer>false</IncludeVsLanguageServer>
   </PropertyGroup>
 
 </Project>

src/fsharp/IlxGen.fs

@@ -826,8 +826,15 @@ and IlxGenEnv =
 
       /// Are we under the scope of a try, catch or finally? If so we can't tailcall. SEH = structured exception handling
       withinSEH: bool
+
+      /// Are we inside of a recursive let binding, while loop, or a for loop?
+      isInLoop: bool
     }
 
+let SetIsInLoop isInLoop eenv =
+    if eenv.isInLoop = isInLoop then eenv
+    else { eenv with isInLoop = isInLoop }
+
 let ReplaceTyenv tyenv (eenv: IlxGenEnv) = {eenv with tyenv = tyenv }
 
 let EnvForTypars tps eenv = {eenv with tyenv = TypeReprEnv.ForTypars tps }
@@ -3369,6 +3376,7 @@ and GenTryFinally cenv cgbuf eenv (bodyExpr, handlerExpr, m, resty, spTry, spFin
 //--------------------------------------------------------------------------
 
 and GenForLoop cenv cgbuf eenv (spFor, v, e1, dir, e2, loopBody, m) sequel =
+    let eenv = SetIsInLoop true eenv
     let g = cenv.g
 
     // The JIT/NGen eliminate array-bounds checks for C# loops of form:
@@ -3459,6 +3467,7 @@ and GenForLoop cenv cgbuf eenv (spFor, v, e1, dir, e2, loopBody, m) sequel =
 //--------------------------------------------------------------------------
 
 and GenWhileLoop cenv cgbuf eenv (spWhile, e1, e2, m) sequel =
+    let eenv = SetIsInLoop true eenv
     let finish = CG.GenerateDelayMark cgbuf "while_finish"
     let startTest = CG.GenerateMark cgbuf "startTest"
 
@@ -5083,6 +5092,7 @@ and GenLetRecFixup cenv cgbuf eenv (ilxCloSpec: IlxClosureSpec, e, ilField: ILFi
 
 /// Generate letrec bindings
 and GenLetRecBindings cenv (cgbuf: CodeGenBuffer) eenv (allBinds: Bindings, m) =
+    let eenv = SetIsInLoop true eenv
     // Fix up recursion for non-toplevel recursive bindings
     let bindsPossiblyRequiringFixup =
         allBinds |> List.filter (fun b ->
@@ -5324,8 +5334,8 @@ and GenBindingAfterSequencePoint cenv cgbuf eenv sp (TBind(vspec, rhsExpr, _)) s
     | _ ->
         let storage = StorageForVal cenv.g m vspec eenv
         match storage, rhsExpr with
-        // locals are zero-init, no need to initialize them
-        | Local (_, realloc, _), Expr.Const (Const.Zero, _, _) when not realloc ->
+        // locals are zero-init, no need to initialize them, except if you are in a loop and the local is mutable.
+        | Local (_, realloc, _), Expr.Const (Const.Zero, _, _) when not realloc && not (eenv.isInLoop && vspec.IsMutable) ->
             CommitStartScope cgbuf startScopeMarkOpt
         | _ ->
             GenBindingRhs cenv cgbuf eenv SPSuppress vspec rhsExpr
@@ -7463,7 +7473,8 @@ let GetEmptyIlxGenEnv (ilg: ILGlobals) ccu =
       liveLocals=IntMap.empty()
       innerVals = []
       sigToImplRemapInfo = [] (* "module remap info" *)
-      withinSEH = false }
+      withinSEH = false
+      isInLoop = false }
 
 type IlxGenResults =
     { ilTypeDefs: ILTypeDef list

src/fsharp/fsi/fsi.fsproj

@@ -33,6 +33,9 @@
     <ProjectReference Include="$(MSBuildThisFileDirectory)..\FSharp.Compiler.Private\FSharp.Compiler.Private.fsproj" />
     <ProjectReference Include="$(MSBuildThisFileDirectory)..\FSharp.Compiler.Interactive.Settings\FSharp.Compiler.Interactive.Settings.fsproj" />
 
+    <!-- only used when '$(TargetFramework)' == 'netstandard2.0' -->
+    <ProjectReference Include="..\FSharp.Build\FSharp.Build.fsproj" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
+
     <!-- only used when '$(TargetFramework)' == 'net472' -->
     <ProjectReference Include="$(MSBuildThisFileDirectory)..\FSharp.Compiler.Server.Shared\FSharp.Compiler.Server.Shared.fsproj" Condition="'$(TargetFramework)' == 'net472'" />
   </ItemGroup>

tests/fsharp/Compiler/Regressions/ForInDoMutableRegressionTest.fs

@@ -0,0 +1,73 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace FSharp.Compiler.UnitTests
+
+open System
+open NUnit.Framework
+
+[<TestFixture()>]
+module ForInDoMutableRegressionTest =
+
+    /// This test is to ensure we initialize locals inside loops.
+    [<Test>]
+    let Script_ForInDoMutableRegressionTest() =
+        let script = 
+            """
+open System.Collections.Generic
+
+let bug() = 
+    for a in [1;2;3;4] do
+      let mutable x = null
+      if x = null then
+        x <- HashSet<int>()
+      x.Add a |> ignore
+      let expected = [a]
+      let actual = List.ofSeq x
+      if expected <> actual then
+        failwith "Bug"
+
+let not_a_bug() = 
+  for a in [1;2;3;4] do
+    let x = ref null
+    if (!x) = null then
+      x := HashSet<int>()
+    (!x).Add a |> ignore
+    let expected = [a]
+    let actual = List.ofSeq (!x)
+    if expected <> actual then
+      failwith "Bug"
+
+let rec test_rec xs =
+    let mutable x = null
+    match xs with
+    | [] -> ()
+    | a :: xs ->
+        if x = null then
+          x <- HashSet<int>()
+        x.Add a |> ignore
+        let expected = [a]
+        let actual = List.ofSeq x
+        if expected <> actual then
+          failwith "Bug"
+        test_rec xs
+
+let test_for_loop () =
+    let xs = [|1;2;3;4|]
+    for i = 0 to xs.Length - 1 do
+      let a = xs.[i]
+      let mutable x = null
+      if x = null then
+        x <- HashSet<int>()
+      x.Add a |> ignore
+      let expected = [a]
+      let actual = List.ofSeq x
+      if expected <> actual then
+        failwith "Bug"
+
+bug ()
+not_a_bug ()
+test_rec [1;2;3;4]
+test_for_loop ()
+            """
+
+        CompilerAssert.RunScript script []

tests/fsharp/FSharpSuite.Tests.fsproj

@@ -37,6 +37,7 @@
     <Compile Include="Compiler\Language\SpanOptimizationTests.fs" />
     <Compile Include="Compiler\Language\SpanTests.fs" />
     <Compile Include="Compiler\Language\StringConcatOptimizationTests.fs" />
+    <Compile Include="Compiler\Regressions\ForInDoMutableRegressionTest.fs" />
     <Content Include="packages.config" />
     <None Include="app.config" />
     <None Include="update.base.line.with.actuals.fsx" />

vsintegration/Vsix/VisualFSharpFull/Source.extension.vsixmanifest

@@ -28,7 +28,10 @@
       <Action Type="Ngen" Path="FSharp.ProjectSystem.FSharp.dll" />
       <Action Type="Ngen" Path="FSharp.ProjectSystem.PropertyPages.dll" />
       <Action Type="Ngen" Path="FSharp.VS.FSI.dll" />
+      <!-- this Ngen action should be disabled in branch `release/dev16.2`, enabled otherwise -->
+      <!--
       <Action Type="Ngen" Path="Agent\FSharp.Compiler.LanguageServer.exe" />
+      -->
     </Actions>
   </Installer>
   <Dependencies>

vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs

@@ -24,58 +24,30 @@ open Microsoft.CodeAnalysis.ExternalAccess.FSharp.LanguageServices
 [<AutoOpen>]
 module private FSharpProjectOptionsHelpers =
 
-    let mapCpsProjectToSite(workspace:VisualStudioWorkspace, project:Project, serviceProvider:System.IServiceProvider, cpsCommandLineOptions: IDictionary<ProjectId, string[] * string[]>) =
-        let hier = workspace.GetHierarchy(project.Id)
+    let mapCpsProjectToSite(project:Project, cpsCommandLineOptions: IDictionary<ProjectId, string[] * string[]>) =
         let sourcePaths, referencePaths, options =
             match cpsCommandLineOptions.TryGetValue(project.Id) with
             | true, (sourcePaths, options) -> sourcePaths, [||], options
             | false, _ -> [||], [||], [||]
+        let mutable errorReporter = Unchecked.defaultof<_>
         {
-            new IProvideProjectSite with
-                member x.GetProjectSite() =
-                    let mutable errorReporter = 
-                        let reporter = ProjectExternalErrorReporter(project.Id, "FS", serviceProvider)
-                        Some(reporter:> IVsLanguageServiceBuildErrorReporter2)
-
-                    {
-                        new IProjectSite with
-                            member __.Description = project.Name
-                            member __.CompilationSourceFiles = sourcePaths
-                            member __.CompilationOptions =
-                                Array.concat [options; referencePaths |> Array.map(fun r -> "-r:" + r)]
-                            member __.CompilationReferences = referencePaths
-                            member site.CompilationBinOutputPath = site.CompilationOptions |> Array.tryPick (fun s -> if s.StartsWith("-o:") then Some s.[3..] else None)
-                            member __.ProjectFileName = project.FilePath
-                            member __.AdviseProjectSiteChanges(_,_) = ()
-                            member __.AdviseProjectSiteCleaned(_,_) = ()
-                            member __.AdviseProjectSiteClosed(_,_) = ()
-                            member __.IsIncompleteTypeCheckEnvironment = false
-                            member __.TargetFrameworkMoniker = ""
-                            member __.ProjectGuid =  project.Id.Id.ToString()
-                            member __.LoadTime = System.DateTime.Now
-                            member __.ProjectProvider = Some (x)
-                            member __.BuildErrorReporter with get () = errorReporter and set (v) = errorReporter <- v
-                    }
-            interface IVsHierarchy with
-                member __.SetSite(psp) = hier.SetSite(psp)
-                member __.GetSite(psp) = hier.GetSite(ref psp)
-                member __.QueryClose(pfCanClose)= hier.QueryClose(ref pfCanClose)
-                member __.Close() = hier.Close()
-                member __.GetGuidProperty(itemid, propid, pguid) = hier.GetGuidProperty(itemid, propid, ref pguid)
-                member __.SetGuidProperty(itemid, propid, rguid) = hier.SetGuidProperty(itemid, propid, ref rguid)
-                member __.GetProperty(itemid, propid, pvar) = hier.GetProperty(itemid, propid, ref pvar) 
-                member __.SetProperty(itemid, propid, var)  = hier.SetProperty(itemid, propid, var)
-                member __.GetNestedHierarchy(itemid, iidHierarchyNested, ppHierarchyNested, pitemidNested) = 
-                    hier.GetNestedHierarchy(itemid, ref iidHierarchyNested, ref ppHierarchyNested, ref pitemidNested)
-                member __.GetCanonicalName(itemid, pbstrName) = hier.GetCanonicalName(itemid, ref pbstrName)
-                member __.ParseCanonicalName(pszName, pitemid) = hier.ParseCanonicalName(pszName, ref pitemid)
-                member __.Unused0() = hier.Unused0()
-                member __.AdviseHierarchyEvents(pEventSink, pdwCookie) = hier.AdviseHierarchyEvents(pEventSink, ref pdwCookie)
-                member __.UnadviseHierarchyEvents(dwCookie) = hier.UnadviseHierarchyEvents(dwCookie)
-                member __.Unused1() = hier.Unused1()
-                member __.Unused2() = hier.Unused2()
-                member __.Unused3() = hier.Unused3()
-                member __.Unused4() = hier.Unused4()
+            new IProjectSite with
+                member __.Description = project.Name
+                member __.CompilationSourceFiles = sourcePaths
+                member __.CompilationOptions =
+                    Array.concat [options; referencePaths |> Array.map(fun r -> "-r:" + r)]
+                member __.CompilationReferences = referencePaths
+                member site.CompilationBinOutputPath = site.CompilationOptions |> Array.tryPick (fun s -> if s.StartsWith("-o:") then Some s.[3..] else None)
+                member __.ProjectFileName = project.FilePath
+                member __.AdviseProjectSiteChanges(_,_) = ()
+                member __.AdviseProjectSiteCleaned(_,_) = ()
+                member __.AdviseProjectSiteClosed(_,_) = ()
+                member __.IsIncompleteTypeCheckEnvironment = false
+                member __.TargetFrameworkMoniker = ""
+                member __.ProjectGuid =  project.Id.Id.ToString()
+                member __.LoadTime = System.DateTime.Now
+                member __.ProjectProvider = None
+                member __.BuildErrorReporter with get () = errorReporter and set (v) = errorReporter <- v
         }
 
     let hasProjectVersionChanged (oldProject: Project) (newProject: Project) =
@@ -108,11 +80,13 @@ type private FSharpProjectOptionsMessage =
     | ClearSingleFileOptionsCache of DocumentId
 
 [<Sealed>]
-type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, settings: EditorOptions, serviceProvider, checkerProvider: FSharpCheckerProvider) =
+type private FSharpProjectOptionsReactor (_workspace: VisualStudioWorkspace, settings: EditorOptions, _serviceProvider, checkerProvider: FSharpCheckerProvider) =
     let cancellationTokenSource = new CancellationTokenSource()
 
     // Hack to store command line options from HandleCommandLineChanges
-    let cpsCommandLineOptions = new ConcurrentDictionary<ProjectId, string[] * string[]>()
+    let cpsCommandLineOptions = ConcurrentDictionary<ProjectId, string[] * string[]>()
+
+    let legacyProjectSites = ConcurrentDictionary<ProjectId, IProjectSite>()
 
     let cache = Dictionary<ProjectId, Project * FSharpParsingOptions * FSharpProjectOptions>()
     let singleFileCache = Dictionary<DocumentId, VersionStamp * FSharpParsingOptions * FSharpProjectOptions>()
@@ -158,6 +132,16 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
                 else
                     return Some(parsingOptions, projectOptions)
         }
+
+    let tryGetProjectSite (project: Project) =
+        // Cps
+        if cpsCommandLineOptions.ContainsKey project.Id then
+            Some (mapCpsProjectToSite(project, cpsCommandLineOptions))
+        else
+            // Legacy
+            match legacyProjectSites.TryGetValue project.Id with
+            | true, site -> Some site
+            | _ -> None
 
     let rec tryComputeOptions (project: Project) =
         async {
@@ -183,15 +167,9 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
                     return None
                 else
 
-                let hier = workspace.GetHierarchy(projectId)
-                let projectSite = 
-                    match hier with
-                    // Legacy
-                    | (:? IProvideProjectSite as provideSite) -> provideSite.GetProjectSite()
-                    // Cps
-                    | _ -> 
-                        let provideSite = mapCpsProjectToSite(workspace, project, serviceProvider, cpsCommandLineOptions)
-                        provideSite.GetProjectSite()
+                match tryGetProjectSite project with
+                | None -> return None
+                | Some projectSite ->             
 
                 let otherOptions =
                     project.ProjectReferences
@@ -283,6 +261,7 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
 
                 | FSharpProjectOptionsMessage.ClearOptions(projectId) ->
                     cache.Remove(projectId) |> ignore
+                    legacyProjectSites.TryRemove(projectId) |> ignore
                 | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) ->
                     singleFileCache.Remove(documentId) |> ignore
         }
@@ -304,6 +283,9 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
     member __.SetCpsCommandLineOptions(projectId, sourcePaths, options) =
         cpsCommandLineOptions.[projectId] <- (sourcePaths, options)
 
+    member __.SetLegacyProjectSite (projectId, projectSite) =
+        legacyProjectSites.[projectId] <- projectSite
+
     member __.TryGetCachedOptionsByProjectId(projectId) =
         match cache.TryGetValue(projectId) with
         | true, result -> Some(result)
@@ -344,6 +326,9 @@ type internal FSharpProjectOptionsManager
             | _ -> ()
         )
 
+    member __.SetLegacyProjectSite (projectId, projectSite) =
+        reactor.SetLegacyProjectSite (projectId, projectSite)
+
     /// Clear a project from the project table
     member this.ClearInfoForProject(projectId:ProjectId) = 
         reactor.ClearOptionsByProjectId(projectId)

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

@@ -19,6 +19,7 @@ open Microsoft.VisualStudio.LanguageServices.ProjectSystem
 open Microsoft.VisualStudio.Shell
 open Microsoft.VisualStudio.Shell.Interop
 open Microsoft.VisualStudio.Text.Outlining
+open FSharp.NativeInterop
 open Microsoft.CodeAnalysis.ExternalAccess.FSharp
 
 #nowarn "9" // NativePtr.toNativeInt

vsintegration/src/FSharp.Editor/LanguageService/LegacyProjectWorkspaceMap.fs

@@ -46,6 +46,8 @@ type internal LegacyProjectWorkspaceMap(solution: IVsSolution,
 
         let projectId = projectContext.Id
 
+        projectInfoManager.SetLegacyProjectSite (projectId, site)
+
         // Sync the source files in projectContext.  Note that these source files are __not__ maintained in order in projectContext
         // as edits are made. It seems this is ok because the source file list is only used to drive roslyn per-file checking.
         let updatedFiles = site.CompilationSourceFiles |> wellFormedFilePathSetIgnoreCase

vsintegration/src/FSharp.Editor/QuickInfo/QuickInfoProvider.fs

@@ -164,7 +164,7 @@ type internal FSharpAsyncQuickInfoSource
         checkerProvider:FSharpCheckerProvider,
         projectInfoManager:FSharpProjectOptionsManager,
         textBuffer:ITextBuffer,
-        settings: EditorOptions
+        _settings: EditorOptions
     ) =
 
     static let joinWithLineBreaks segments =
@@ -206,9 +206,10 @@ type internal FSharpAsyncQuickInfoSource
         // This method can be called from the background thread.
         // Do not call IServiceProvider.GetService here.
         override __.GetQuickInfoItemAsync(session:IAsyncQuickInfoSession, cancellationToken:CancellationToken) : Task<QuickInfoItem> =
-            // if using LSP, just bail early
-            if settings.Advanced.UsePreviewTextHover then Task.FromResult<QuickInfoItem>(null)
-            else
+            // The following lines should be disabled for branch `release/dev16.2`, enabled otherwise
+            //// if using LSP, just bail early
+            //if settings.Advanced.UsePreviewTextHover then Task.FromResult<QuickInfoItem>(null)
+            //else
             let triggerPoint = session.GetTriggerPoint(textBuffer.CurrentSnapshot)
             match triggerPoint.HasValue with
             | false -> Task.FromResult<QuickInfoItem>(null)