Fix solution reload and config change issues (#3025)

* fix config change

* Fix configuration changes

* Fix colorization cache for changing defines

* fix config switching

* add manual test cases

* Fix test

* minor nit

* use correct reference count

* cleanup decrement logic

* fix build

* Update CompileOps.fs

* adjust unit tests mocks

* adjust unit tests mocks

* fix test

* add new misc project test

* do ComputeSourcesAndFlags later

* do ComputeSourcesAndFlags once

* variations

* keep dialog open

fcs/FSharp.Compiler.Service.ProjectCracker/ProjectCracker.fs

@@ -33,7 +33,7 @@ type ProjectCracker =
 
             logMap := Map.add opts.ProjectFile opts.LogOutput !logMap
             { ProjectFileName = opts.ProjectFile
-              ProjectFileNames = sourceFiles
+              SourceFiles = sourceFiles
               OtherOptions = otherOptions
               ReferencedProjects = referencedProjects
               IsIncompleteTypeCheckEnvironment = false

src/absil/ilread.fs

@@ -3967,7 +3967,7 @@ let OpenILModuleReader infile opts =
 
 // ++GLOBAL MUTABLE STATE (concurrency safe via locking)
 type ILModuleReaderCacheLockToken() = interface LockToken
-let ilModuleReaderCache = new AgedLookup<ILModuleReaderCacheLockToken, (string * System.DateTime * ILScopeRef * bool), ILModuleReader>(0, areSame=(fun (x,y) -> x = y))
+let ilModuleReaderCache = new AgedLookup<ILModuleReaderCacheLockToken, (string * System.DateTime * ILScopeRef * bool), ILModuleReader>(0, areSimilar=(fun (x,y) -> x = y))
 let ilModuleReaderCacheLock = Lock()
 
 let OpenILModuleReaderAfterReadingAllBytes infile opts = 

src/fsharp/CompileOps.fs

@@ -2701,13 +2701,15 @@ type TcConfig private (data : TcConfigBuilder,validate:bool) =
         match fslibExplicitFilenameOpt with
         | Some(fslibFilename) ->
             let filename = ComputeMakePathAbsolute data.implicitIncludeDir fslibFilename
-            try 
-                use ilReader = OpenILBinary(filename,data.optimizeForMemory,data.openBinariesInMemory,None,None, data.shadowCopyReferences)
-                checkFSharpBinaryCompatWithMscorlib filename ilReader.ILAssemblyRefs ilReader.ILModuleDef.ManifestOfAssembly.Version rangeStartup;
-                let fslibRoot = Path.GetDirectoryName(FileSystem.GetFullPathShim(filename))
-                fslibRoot (* , sprintf "v%d.%d" v1 v2 *)
-            with e -> 
-                error(Error(FSComp.SR.buildErrorOpeningBinaryFile(filename, e.Message), rangeStartup))
+            if fslibReference.ProjectReference.IsNone then 
+                try 
+                    use ilReader = OpenILBinary(filename,data.optimizeForMemory,data.openBinariesInMemory,None,None, data.shadowCopyReferences)
+                    checkFSharpBinaryCompatWithMscorlib filename ilReader.ILAssemblyRefs ilReader.ILModuleDef.ManifestOfAssembly.Version rangeStartup;
+                with e -> 
+                    error(Error(FSComp.SR.buildErrorOpeningBinaryFile(filename, e.Message), rangeStartup))
+
+            let fslibRoot = Path.GetDirectoryName(FileSystem.GetFullPathShim(filename))
+            fslibRoot
         | _ ->
             data.defaultFSharpBinariesDir
 #endif

src/fsharp/InternalCollections.fs

@@ -13,7 +13,7 @@ type internal ValueStrength<'T when 'T : not struct> =
    | Weak of WeakReference<'T>
 #endif
 
-type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStrongly:int, areSame, ?requiredToKeep, ?onStrongDiscard, ?keepMax: int) =
+type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStrongly:int, areSimilar, ?requiredToKeep, ?onStrongDiscard, ?keepMax: int) =
     /// The list of items stored. Youngest is at the end of the list.
     /// The choice of order is somewhat arbitrary. If the other way then adding
     /// items would be O(1) and removing O(N).
@@ -39,8 +39,8 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro
             // This function returns true if two keys are the same according to the predicate
             // function passed in.
             | []->None
-            | (key',value)::t->
-                if areSame(key,key') then Some(key',value) 
+            | (similarKey,value)::t->
+                if areSimilar(key,similarKey) then Some(similarKey,value) 
                 else Lookup key t      
         Lookup key data    
 
@@ -53,18 +53,18 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro
 
     /// Promote a particular key value.
     let Promote (data, key, value) = 
-        (data |> List.filter (fun (key',_)-> not (areSame(key,key')))) @ [ (key, value) ] 
+        (data |> List.filter (fun (similarKey,_)-> not (areSimilar(key,similarKey)))) @ [ (key, value) ] 
 
     /// Remove a particular key value.
     let RemoveImpl (data, key) = 
-        let discard,keep = data |> List.partition (fun (key',_)-> areSame(key,key'))
+        let discard,keep = data |> List.partition (fun (similarKey,_)-> areSimilar(key,similarKey))
         keep, discard
 
     let TryGetKeyValueImpl(data,key) = 
         match TryPeekKeyValueImpl(data,key) with 
-        | Some(key', value) as result ->
+        | Some(similarKey, value) as result ->
             // If the result existed, move it to the end of the list (more likely to keep it)
-            result,Promote (data,key',value)
+            result,Promote (data,similarKey,value)
         | None -> None,data          
 
     /// Remove weak entries from the list that have been collected.
@@ -154,37 +154,42 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro
 
 
 
-type internal MruCache<'Token, 'Key,'Value when 'Value : not struct>(keepStrongly, areSame, ?isStillValid : 'Key*'Value->bool, ?areSameForSubsumption, ?requiredToKeep, ?onStrongDiscard, ?keepMax) =
+type internal MruCache<'Token, 'Key,'Value when 'Value : not struct>(keepStrongly, areSame, ?isStillValid : 'Key*'Value->bool, ?areSimilar, ?requiredToKeep, ?onStrongDiscard, ?keepMax) =
 
-    /// Default behavior of <c>areSameForSubsumption</c> function is areSame.
-    let areSameForSubsumption = defaultArg areSameForSubsumption areSame
+    /// Default behavior of <c>areSimilar</c> function is areSame.
+    let areSimilar = defaultArg areSimilar areSame
 
     /// The list of items in the cache. Youngest is at the end of the list.
     /// The choice of order is somewhat arbitrary. If the other way then adding
     /// items would be O(1) and removing O(N).
-    let cache = AgedLookup<'Token, 'Key,'Value>(keepStrongly=keepStrongly,areSame=areSameForSubsumption,?onStrongDiscard=onStrongDiscard,?keepMax=keepMax,?requiredToKeep=requiredToKeep)
+    let cache = AgedLookup<'Token, 'Key,'Value>(keepStrongly=keepStrongly,areSimilar=areSimilar,?onStrongDiscard=onStrongDiscard,?keepMax=keepMax,?requiredToKeep=requiredToKeep)
 
     /// Whether or not this result value is still valid.
     let isStillValid = defaultArg isStillValid (fun _ -> true)
 
+    member bc.ContainsSimilarKey(tok, key) = 
+        match cache.TryPeekKeyValue(tok, key) with
+        | Some(_similarKey, _value)-> true
+        | None -> false
+
     member bc.TryGetAny(tok, key) = 
         match cache.TryPeekKeyValue(tok, key) with
-        | Some(key', value)->
-            if areSame(key',key) then Some(value)
+        | Some(similarKey, value)->
+            if areSame(similarKey,key) then Some(value)
             else None
         | None -> None
 
     member bc.TryGet(tok, key) = 
         match cache.TryGetKeyValue(tok, key) with
-        | Some(key', value) -> 
-            if areSame(key', key) && isStillValid(key,value) then Some value
+        | Some(similarKey, value) -> 
+            if areSame(similarKey, key) && isStillValid(key,value) then Some value
             else None
         | None -> None
 
     member bc.Set(tok, key:'Key,value:'Value) = 
         cache.Put(tok, key,value)
 
-    member bc.Remove(tok, key) = 
+    member bc.RemoveAnySimilar(tok, key) = 
         cache.Remove(tok, key)
 
     member bc.Clear(tok) =

src/fsharp/InternalCollections.fsi

@@ -4,10 +4,12 @@ namespace Internal.Utilities.Collections
 
   /// Simple aging lookup table. When a member is accessed it's
   /// moved to the top of the list and when there are too many elements
-  /// the least-recently-accessed element falls of the end.
+  /// the least-recently-accessed element falls of the end.  
+  ///
+  ///  - areSimilar: Keep at most once association for two similar keys (as given by areSimilar)
   type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct> = 
     new : keepStrongly:int
-            * areSame:('Key * 'Key -> bool) 
+            * areSimilar:('Key * 'Key -> bool) 
             * ?requiredToKeep:('Value -> bool)
             * ?onStrongDiscard : ('Value -> unit) // this may only be set if keepTotal=keepStrongly, i.e. not weak entries
             * ?keepMax: int
@@ -40,25 +42,36 @@ namespace Internal.Utilities.Collections
   /// threads seeing different live sets of cached items, and may result in the onDiscard action
   /// being called multiple times. In practice this means the collection is only safe for concurrent
   /// access if there is no discard action to execute.
+  ///
+  ///  - areSimilar: Keep at most once association for two similar keys (as given by areSimilar)
   type internal MruCache<'Token, 'Key,'Value when 'Value : not struct> =
     new : keepStrongly:int 
             * areSame:('Key * 'Key -> bool) 
             * ?isStillValid:('Key * 'Value -> bool)
-            * ?areSameForSubsumption:('Key * 'Key -> bool) 
+            * ?areSimilar:('Key * 'Key -> bool) 
             * ?requiredToKeep:('Value -> bool)
             * ?onDiscard:('Value -> unit)
             * ?keepMax:int
             -> MruCache<'Token,'Key,'Value>
+
     /// Clear out the cache.
     member Clear : 'Token -> unit
-    /// Get the value for the given key or <c>None</c> if not already available.
+
+    /// Get the similar (subsumable) value for the given key or <c>None</c> if not already available.
+    member ContainsSimilarKey : 'Token * key:'Key -> bool
+
+    /// Get the value for the given key or <c>None</c> if not still valid.
     member TryGetAny : 'Token * key:'Key -> 'Value option
-    /// Get the value for the given key or None if not already available
+
+    /// Get the value for the given key or None, but only if entry is still valid
     member TryGet : 'Token * key:'Key -> 'Value option
+
     /// Remove the given value from the mru cache.
-    member Remove : 'Token * key:'Key -> unit
+    member RemoveAnySimilar : 'Token * key:'Key -> unit
+
     /// Set the given key. 
     member Set : 'Token * key:'Key * value:'Value -> unit
+
     /// Resize
     member Resize : 'Token * keepStrongly: int * ?keepMax : int -> unit
 

src/fsharp/vs/IncrementalBuild.fs

@@ -1037,7 +1037,7 @@ type FrameworkImportsCacheKey = (*resolvedpath*)string list * string * (*TargetF
 type FrameworkImportsCache(keepStrongly) = 
 
     // Mutable collection protected via CompilationThreadToken 
-    let frameworkTcImportsCache = AgedLookup<CompilationThreadToken, FrameworkImportsCacheKey,(TcGlobals * TcImports)>(keepStrongly, areSame=(fun (x,y) -> x = y)) 
+    let frameworkTcImportsCache = AgedLookup<CompilationThreadToken, FrameworkImportsCacheKey,(TcGlobals * TcImports)>(keepStrongly, areSimilar=(fun (x,y) -> x = y)) 
 
     member __.Downsize(ctok) = frameworkTcImportsCache.Resize(ctok, keepStrongly=0)
     member __.Clear(ctok) = frameworkTcImportsCache.Clear(ctok)
@@ -1562,7 +1562,7 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs
             return true
       }
 
-    member builder.GetCheckResultsBeforeFileInProjectIfReady (filename): PartialCheckResults option  = 
+    member builder.GetCheckResultsBeforeFileInProjectEvenIfStale (filename): PartialCheckResults option  = 
         let slotOfFile = builder.GetSlotOfFileName filename
         let result = 
             match slotOfFile with
@@ -1676,7 +1676,7 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs
 #endif
       }
 
-    member __.ProjectFileNames  = sourceFiles  |> List.map (fun (_,f,_) -> f)
+    member __.SourceFiles  = sourceFiles  |> List.map (fun (_,f,_) -> f)
 
     /// CreateIncrementalBuilder (for background type checking). Note that fsc.fs also
     /// creates an incremental builder used by the command line compiler.

src/fsharp/vs/IncrementalBuild.fsi

@@ -65,18 +65,14 @@ type internal PartialCheckResults =
 [<Class>]
 type internal IncrementalBuilder = 
 
-      /// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0. The returns an IDisposable which will 
-      /// decrement the usage count on the entire build by 1 and dispose if it is no longer used by anyone.
-      member IncrementUsageCount : unit -> IDisposable
-
       /// Check if the builder is not disposed
       member IsAlive : bool
 
       /// The TcConfig passed in to the builder creation.
       member TcConfig : TcConfig
 
       /// The full set of source files including those from options
-      member ProjectFileNames : string list
+      member SourceFiles : string list
 
       /// Raised just before a file is type-checked, to invalidate the state of the file in VS and force VS to request a new direct typecheck of the file.
       /// The incremental builder also typechecks the file (error and intellisense results from the background builder are not
@@ -110,7 +106,7 @@ type internal IncrementalBuilder =
       /// This is a very quick operation.
       ///
       /// This is safe for use from non-compiler threads but the objects returned must in many cases be accessed only from the compiler thread.
-      member GetCheckResultsBeforeFileInProjectIfReady: filename:string -> PartialCheckResults option
+      member GetCheckResultsBeforeFileInProjectEvenIfStale: filename:string -> PartialCheckResults option
 
       /// Get the preceding typecheck state of a slot, but only if it is up-to-date w.r.t.
       /// the timestamps on files and referenced DLLs prior to this one. Return None if the result is not available.
@@ -153,7 +149,11 @@ type internal IncrementalBuilder =
 
       static member TryCreateBackgroundBuilderForProjectOptions : CompilationThreadToken * ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * FrameworkImportsCache * scriptClosureOptions:LoadClosure option * sourceFiles:string list * commandLineArgs:string list * projectReferences: IProjectReference list * projectDirectory:string * useScriptResolutionRules:bool * keepAssemblyContents: bool * keepAllBackgroundResolutions: bool * maxTimeShareMilliseconds: int64 -> Cancellable<IncrementalBuilder option * FSharpErrorInfo list>
 
+      /// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0 so immediately after creation 
+      /// a call to KeepBuilderAlive should be made. The returns an IDisposable which will 
+      /// decrement the usage count and dispose if the usage count goes to zero
       static member KeepBuilderAlive : IncrementalBuilder option -> IDisposable
+
       member IsBeingKeptAliveApartFromCacheEntry : bool
 
 /// Generalized Incremental Builder. This is exposed only for unittesting purposes.