Some performance optimizations and code cleanup (#16302)

* code optimisations and cleanup:

Some ContainsKey double lookups to TryContainsKey
Some nested list Contains to set Contains.
Some FSharpLint rules
also "dotnet fantomas . -r" but I bet it'll make merging harder.

* Made FSharp.Core.dll expectation tighter. It would be nice to get both SelfContained_Trimming_Test and StaticLinkedFSharpCore_Trimming_Test the results once.

* As expected, StaticLinkedFSharpCore_Trimming_Test smaller too...

* vzarytovskii feedback implemented

* Reverted Sets to Lists due to speculation of memory consumption in the PR comments

Will open this as separate PR

src/Compiler/AbstractIL/il.fs

@@ -5118,7 +5118,7 @@ let decodeILAttribData (ca: ILAttribute) =
                 ILAttribElem.Char(char (int32 n)), sigptr
             | ILType.Value tspec when tspec.Name = "System.Boolean" ->
                 let n, sigptr = sigptr_get_byte bytes sigptr
-                ILAttribElem.Bool(not (n = 0)), sigptr
+                ILAttribElem.Bool(n <> 0), sigptr
             | ILType.Boxed tspec when tspec.Name = "System.String" ->
                 let n, sigptr = sigptr_get_serstring_possibly_null bytes sigptr
                 ILAttribElem.String n, sigptr

src/Compiler/AbstractIL/illex.fsl

@@ -57,16 +57,16 @@ let kwdInstrTable =
    (let t = HashMultiMap(1000, HashIdentity.Structural)
     List.iter t.Add (Lazy.force keywords);
     let addTable f l = List.iter (fun (x,i) -> t.Add (String.concat "." x,f i)) (Lazy.force l)
-    addTable (fun i -> INSTR_NONE i) NoArgInstrs;
-    addTable (fun i -> INSTR_I i) Int32Instrs;
-    addTable (fun i -> INSTR_I32_I32 i) Int32Int32Instrs;
-    addTable (fun i -> INSTR_I8 i) Int64Instrs;
-    addTable (fun i -> INSTR_R i) DoubleInstrs;
-    addTable (fun i -> INSTR_TYPE i) TypeInstrs;
-    addTable (fun i -> INSTR_INT_TYPE i) IntTypeInstrs;
-    addTable (fun i -> INSTR_VALUETYPE i) ValueTypeInstrs;
-    addTable (fun i -> INSTR_STRING i) StringInstrs;
-    addTable (fun i -> INSTR_TOK i) TokenInstrs;
+    addTable (INSTR_NONE) NoArgInstrs;
+    addTable (INSTR_I) Int32Instrs;
+    addTable (INSTR_I32_I32) Int32Int32Instrs;
+    addTable (INSTR_I8) Int64Instrs;
+    addTable (INSTR_R) DoubleInstrs;
+    addTable (INSTR_TYPE) TypeInstrs;
+    addTable (INSTR_INT_TYPE) IntTypeInstrs;
+    addTable (INSTR_VALUETYPE) ValueTypeInstrs;
+    addTable (INSTR_STRING) StringInstrs;
+    addTable (INSTR_TOK) TokenInstrs;
     t)
 
 let kwdOrInstr s = (Lazy.force kwdInstrTable).[s] (* words *)

src/Compiler/AbstractIL/ilnativeres.fs

@@ -54,7 +54,7 @@ type CvtResFile() =
         let mutable resourceNames = List<RESOURCE>()
 
         // The stream might be empty, so let's check
-        if not (reader.PeekChar() = -1) then
+        if reader.PeekChar() <> -1 then
             let mutable startPos = stream.Position
             let mutable initial32Bits = reader.ReadUInt32()
 

src/Compiler/AbstractIL/ilprint.fs

@@ -272,7 +272,7 @@ and output_tyvar os d =
 
 and goutput_typ_with_shortened_class_syntax env os =
     function
-    | ILType.Boxed tspec when tspec.GenericArgs = [] -> goutput_tref env os tspec.TypeRef
+    | ILType.Boxed tspec when List.isEmpty tspec.GenericArgs -> goutput_tref env os tspec.TypeRef
     | typ2 -> goutput_typ env os typ2
 
 and goutput_gactuals env os inst =

src/Compiler/AbstractIL/ilread.fs

@@ -40,14 +40,13 @@ let _ =
 
 let noStableFileHeuristic =
     try
-        (Environment.GetEnvironmentVariable("FSharp_NoStableFileHeuristic") <> null)
+        not (isNull (Environment.GetEnvironmentVariable "FSharp_NoStableFileHeuristic"))
     with _ ->
         false
 
 let alwaysMemoryMapFSC =
     try
-        (Environment.GetEnvironmentVariable("FSharp_AlwaysMemoryMapCommandLineCompiler")
-         <> null)
+        not (isNull (Environment.GetEnvironmentVariable "FSharp_AlwaysMemoryMapCommandLineCompiler"))
     with _ ->
         false
 
@@ -639,9 +638,9 @@ let instrs () =
                  | Some ty -> I_callconstraint(true, tl, ty, mspec, y)
                  | None -> I_callvirt(tl, mspec, y))
          ))
-        i_leave_s, I_unconditional_i8_instr(noPrefixes (fun x -> I_leave x))
+        i_leave_s, I_unconditional_i8_instr(noPrefixes I_leave)
         i_br_s, I_unconditional_i8_instr(noPrefixes I_br)
-        i_leave, I_unconditional_i32_instr(noPrefixes (fun x -> I_leave x))
+        i_leave, I_unconditional_i32_instr(noPrefixes I_leave)
         i_br, I_unconditional_i32_instr(noPrefixes I_br)
         i_brtrue_s, I_conditional_i8_instr(noPrefixes (fun x -> I_brcmp(BI_brtrue, x)))
         i_brfalse_s, I_conditional_i8_instr(noPrefixes (fun x -> I_brcmp(BI_brfalse, x)))
@@ -2826,7 +2825,7 @@ and seekReadMethodDefAsMethodDataUncached ctxtH idx =
         seekReadIndexedRow (
             ctxt.getNumRows TableNames.TypeDef,
             (fun i -> i, seekReadTypeDefRowWithExtents ctxt i),
-            (fun r -> r),
+            id,
             (fun (_, ((_, _, _, _, _, methodsIdx), (_, endMethodsIdx))) ->
                 if endMethodsIdx <= idx then 1
                 elif methodsIdx <= idx && idx < endMethodsIdx then 0
@@ -2874,7 +2873,7 @@ and seekReadFieldDefAsFieldSpecUncached ctxtH idx =
         seekReadIndexedRow (
             ctxt.getNumRows TableNames.TypeDef,
             (fun i -> i, seekReadTypeDefRowWithExtents ctxt i),
-            (fun r -> r),
+            id,
             (fun (_, ((_, _, _, _, fieldsIdx, _), (endFieldsIdx, _))) ->
                 if endFieldsIdx <= idx then 1
                 elif fieldsIdx <= idx && idx < endFieldsIdx then 0

src/Compiler/AbstractIL/ilreflect.fs

@@ -265,12 +265,12 @@ type TypeBuilder with
         let t = typB.CreateTypeAndLog()
 
         let m =
-            if box t <> null then
+            if not (isNull (box t)) then
                 t.GetMethod(nm, (args |> Seq.map (fun x -> x.GetType()) |> Seq.toArray))
             else
                 null
 
-        if m <> null then
+        if not (isNull m) then
             m.Invoke(null, args)
         else
             raise (MissingMethodException nm)
@@ -1787,7 +1787,7 @@ let definePInvokeMethod =
             |]
         )
 
-let enablePInvoke = definePInvokeMethod <> null
+let enablePInvoke = not (isNull definePInvokeMethod)
 
 let rec buildMethodPass2 cenv tref (typB: TypeBuilder) emEnv (mdef: ILMethodDef) =
     let attrs = mdef.Attributes
@@ -1828,7 +1828,7 @@ let rec buildMethodPass2 cenv tref (typB: TypeBuilder) emEnv (mdef: ILMethodDef)
 
         // Use reflection to invoke the api when we are executing on a platform that doesn't directly have this API.
         let methB =
-            System.Diagnostics.Debug.Assert(definePInvokeMethod <> null, "Runtime does not have DefinePInvokeMethod") // Absolutely can't happen
+            System.Diagnostics.Debug.Assert(not (isNull definePInvokeMethod), "Runtime does not have DefinePInvokeMethod") // Absolutely can't happen
 
             definePInvokeMethod.Invoke(
                 typB,
@@ -1988,7 +1988,7 @@ let buildFieldPass2 cenv tref (typB: TypeBuilder) emEnv (fdef: ILFieldDef) =
                     delayedFieldInits = (fun () -> fieldB.SetConstant(initial.AsObject())) :: emEnv.delayedFieldInits
                 }
 
-    fdef.Offset |> Option.iter (fun offset -> fieldB.SetOffset offset)
+    fdef.Offset |> Option.iter (fieldB.SetOffset)
     // custom attributes: done on pass 3 as they may reference attribute constructors generated on
     // pass 2.
     let fref = mkILFieldRef (tref, fdef.Name, fdef.FieldType)

src/Compiler/AbstractIL/ilsign.fs

@@ -183,18 +183,18 @@ let toCLRKeyBlob (rsaParameters: RSAParameters) (algId: int) : byte array =
         raise (CryptographicException(getResourceString (FSComp.SR.ilSignInvalidAlgId ())))
 
     // Validate the RSA structure first.
-    if rsaParameters.Modulus = null then
+    if isNull rsaParameters.Modulus then
         raise (CryptographicException(String.Format(getResourceString (FSComp.SR.ilSignInvalidRSAParams ()), "Modulus")))
 
-    if rsaParameters.Exponent = null || rsaParameters.Exponent.Length > 4 then
+    if isNull rsaParameters.Exponent || rsaParameters.Exponent.Length > 4 then
         raise (CryptographicException(String.Format(getResourceString (FSComp.SR.ilSignInvalidRSAParams ()), "Exponent")))
 
     let modulusLength = rsaParameters.Modulus.Length
     let halfModulusLength = (modulusLength + 1) / 2
 
     // We assume that if P != null, then so are Q, DP, DQ, InverseQ and D and indicate KeyPair RSA Parameters
     let isPrivate =
-        if rsaParameters.P <> null then
+        if not (isNull rsaParameters.P) then
             validateRSAField rsaParameters.P halfModulusLength "P"
             validateRSAField rsaParameters.Q halfModulusLength "Q"
             validateRSAField rsaParameters.DP halfModulusLength "DP"
@@ -213,7 +213,7 @@ let toCLRKeyBlob (rsaParameters: RSAParameters) (algId: int) : byte array =
         bw.Write(int (modulusLength + BLOBHEADER_LENGTH)) // CLRHeader.KeyLength
 
         // Write out the BLOBHEADER
-        bw.Write(byte (if isPrivate = true then PRIVATEKEYBLOB else PUBLICKEYBLOB)) // BLOBHEADER.bType
+        bw.Write(byte (if isPrivate then PRIVATEKEYBLOB else PUBLICKEYBLOB)) // BLOBHEADER.bType
 
         bw.Write(byte BLOBHEADER_CURRENT_BVERSION) // BLOBHEADER.bVersion
         bw.Write(int16 0) // BLOBHEADER.wReserved
@@ -235,7 +235,7 @@ let toCLRKeyBlob (rsaParameters: RSAParameters) (algId: int) : byte array =
         bw.Write expAsDword // RSAPubKey.pubExp
         bw.Write(rsaParameters.Modulus |> Array.rev) // Copy over the modulus for both public and private
 
-        if isPrivate = true then
+        if isPrivate then
             do
                 bw.Write(rsaParameters.P |> Array.rev)
                 bw.Write(rsaParameters.Q |> Array.rev)

src/Compiler/AbstractIL/ilwritepdb.fs

@@ -324,7 +324,7 @@ let sortMethods info =
 
 let getRowCounts tableRowCounts =
     let builder = ImmutableArray.CreateBuilder<int>(tableRowCounts |> Array.length)
-    tableRowCounts |> Seq.iter (fun x -> builder.Add x)
+    tableRowCounts |> Seq.iter (builder.Add)
     builder.MoveToImmutable()
 
 let scopeSorter (scope1: PdbMethodScope) (scope2: PdbMethodScope) =

src/Compiler/Checking/CheckDeclarations.fs

@@ -1728,7 +1728,7 @@ module MutRecBindingChecking =
                      // Check it is not one of the generalized variables...
                      not (genSet.Contains tp) && 
                      // Check it involves a generalized variable in one of its constraints...
-                     freeInTypar.Exists(fun otherTypar -> genSet.Contains otherTypar))
+                     freeInTypar.Exists(genSet.Contains))
              //printfn "unsolvedTyparsInvolvingGeneralizedVariables.Length = %d" unsolvedTyparsInvolvingGeneralizedVariables.Length
              //for x in unsolvedTypars do 
              //    printfn "unsolvedTyparsInvolvingGeneralizedVariable: %s #%d" x.DisplayName x.Stamp
@@ -2536,16 +2536,17 @@ module EstablishTypeDefinitionCores =
                 error(Error(FSComp.SR.tcKindOfTypeSpecifiedDoesNotMatchDefinition(), m))
             k
 
+    [<return: Struct>]
     let private (|TyconCoreAbbrevThatIsReallyAUnion|_|) (hasMeasureAttr, envinner: TcEnv, id: Ident) synTyconRepr =
         match synTyconRepr with 
         | SynTypeDefnSimpleRepr.TypeAbbrev(_, StripParenTypes (SynType.LongIdent(SynLongIdent([unionCaseName], _, _))), m) 
                               when 
                                 (not hasMeasureAttr && 
                                  (isNil (LookupTypeNameInEnvNoArity OpenQualified unionCaseName.idText envinner.NameEnv) || 
                                   id.idText = unionCaseName.idText)) -> 
-            Some(unionCaseName, m)
+            ValueSome(unionCaseName, m)
         | _ -> 
-            None
+            ValueNone
 
     /// Get the component types that make a record, union or struct type.
     ///
@@ -5321,7 +5322,7 @@ and TcModuleOrNamespaceElements cenv parent endm env xml mutRecNSInfo openDecls0
         let moduleContents = TMDefs moduleDefs 
 
         // Collect up the attributes that are global to the file 
-        let topAttrsNew = compiledDefs |> List.map p33 |> List.concat
+        let topAttrsNew = compiledDefs |> List.collect p33
         return (moduleContents, topAttrsNew, envAtEnd)
   }  
 
@@ -5412,7 +5413,7 @@ let CombineTopAttrs topAttrs1 topAttrs2 =
       assemblyAttrs = topAttrs1.assemblyAttrs @ topAttrs2.assemblyAttrs } 
 
 let rec IterTyconsOfModuleOrNamespaceType f (mty: ModuleOrNamespaceType) = 
-    mty.AllEntities |> QueueList.iter (fun tycon -> f tycon)
+    mty.AllEntities |> QueueList.iter f
     mty.ModuleAndNamespaceDefinitions |> List.iter (fun v -> 
         IterTyconsOfModuleOrNamespaceType f v.ModuleOrNamespaceType)
 

src/Compiler/Checking/CheckExpressions.fs

@@ -1118,7 +1118,7 @@ let PublishValueDefn (cenv: cenv) env declKind (vspec: Val) =
     | Some _ when
         (not vspec.IsCompilerGenerated &&
          // Extrinsic extensions don't get added to the tcaug
-         not (declKind = ExtrinsicExtensionBinding)) ->
+         declKind <> ExtrinsicExtensionBinding) ->
          // // Static initializers don't get published to the tcaug
          // not (memberInfo.MemberFlags.MemberKind = SynMemberKind.ClassConstructor)) ->
 
@@ -1223,10 +1223,11 @@ let CheckRequiredProperties (g:TcGlobals) (env: TcEnv) (cenv: TcFileState) (minf
             let setterPropNames =
                 finalAssignedItemSetters
                 |> List.choose (function | AssignedItemSetter(_, AssignedPropSetter (_, pinfo, _, _), _) -> Some pinfo.PropertyName  | _ -> None)
+                |> Set.ofList
 
             let missingProps =
                 requiredProps
-                |> List.filter (fun pinfo -> not (List.contains pinfo.PropertyName setterPropNames))
+                |> List.filter (fun pinfo -> not (Set.contains pinfo.PropertyName setterPropNames))
             if missingProps.Length > 0 then
                 let details = NicePrint.multiLineStringOfPropInfos g cenv.amap mMethExpr env.DisplayEnv missingProps
                 errorR(Error(FSComp.SR.tcMissingRequiredMembers details, mMethExpr))
@@ -4774,7 +4775,7 @@ and TcStaticConstantParameter (cenv: cenv) (env: TcEnv) tpenv kind (StripParenTy
             | SynConst.Double n when typeEquiv g g.float_ty kind -> record(g.float_ty); box (n: double)
             | SynConst.Char n when typeEquiv g g.char_ty kind -> record(g.char_ty); box (n: char)
             | SynConst.String (s, _, _)
-            | SynConst.SourceIdentifier (_, s, _) when s <> null && typeEquiv g g.string_ty kind -> record(g.string_ty); box (s: string)
+            | SynConst.SourceIdentifier (_, s, _) when (not (isNull s)) && typeEquiv g g.string_ty kind -> record(g.string_ty); box (s: string)
             | SynConst.Bool b when typeEquiv g g.bool_ty kind -> record(g.bool_ty); box (b: bool)
             | _ -> fail()
         v, tpenv
@@ -5715,7 +5716,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
         cenv.TcArrayOrListComputedExpression cenv env overallTy tpenv (isArray, comp)  m
 
     | SynExpr.LetOrUse _ ->
-        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr (fun x -> x)
+        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr id
 
     | SynExpr.TryWith (synBodyExpr, synWithClauses, mTryToLast, spTry, spWith, trivia) ->
         TcExprTryWith cenv overallTy env tpenv (synBodyExpr, synWithClauses, trivia.WithToEndRange, mTryToLast, spTry, spWith)
@@ -5747,7 +5748,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
         TcStmtThatCantBeCtorBody cenv env tpenv synInnerExpr
 
     | SynExpr.IfThenElse _ ->
-        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr (fun x -> x)
+        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr id
 
     // This is for internal use in the libraries only
     | SynExpr.LibraryOnlyStaticOptimization (constraints, expr2, expr3, m) ->
@@ -6088,7 +6089,7 @@ and TcExprJoinIn (cenv: cenv) overallTy env tpenv (synExpr1, mInToken, synExpr2,
 
 and TcExprSequential (cenv: cenv) overallTy env tpenv (synExpr, _sp, dir, synExpr1, synExpr2, m) =
     if dir then
-        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr (fun x -> x)
+        TcLinearExprs (TcExprThatCanBeCtorBody cenv) cenv env overallTy tpenv false synExpr id
     else
         // Constructors using "new (...) = <ctor-expr> then <expr>"
         let env = { env with eIsControlFlow = true }
@@ -11049,7 +11050,7 @@ and TcLetBinding (cenv: cenv) isUse env containerInfo declKind tpenv (synBinds,
     let lazyFreeInEnv = lazy (GeneralizationHelpers.ComputeUngeneralizableTypars env)
 
     // Generalize the bindings...
-    (((fun x -> x), env, tpenv), checkedBinds) ||> List.fold (fun (buildExpr, env, tpenv) tbinfo ->
+    ((id, env, tpenv), checkedBinds) ||> List.fold (fun (buildExpr, env, tpenv) tbinfo ->
         let (CheckedBindingInfo(inlineFlag, attrs, xmlDoc, tcPatPhase2, explicitTyparInfo, nameToPrelimValSchemeMap, rhsExpr, _, tauTy, m, debugPoint, _, literalValue, isFixed)) = tbinfo
         let enclosingDeclaredTypars = []
         let (ExplicitTyparInfo(_, declaredTypars, canInferTypars)) = explicitTyparInfo
@@ -12311,7 +12312,7 @@ and TcLetrecBindings overridesOK (cenv: cenv) env tpenv (binds, bindsm, scopem)
              bindsWithoutLaziness
              //(fun
              (fun doBindings bindings -> doBindings bindings)
-             (fun bindings -> bindings)
+             id
              (fun doBindings bindings -> [doBindings bindings])
              bindsm
         List.concat results

src/Compiler/Checking/CheckPatterns.fs

@@ -387,7 +387,7 @@ and TcPatOr warnOnUpper cenv env vFlags patEnv ty pat1 pat2 m =
     let pat2R, patEnv2 = TcPat warnOnUpper cenv env None vFlags (TcPatLinearEnv(tpenv, names, takenNames)) ty pat2
     let (TcPatLinearEnv(tpenv, names2, takenNames2)) = patEnv2
 
-    if not (takenNames1 = takenNames2) then
+    if takenNames1 <> takenNames2 then
         errorR (UnionPatternsBindDifferentNames m)
 
     names1 |> Map.iter (fun _ (PrelimVal1 (id=id1; prelimType=ty1)) ->

src/Compiler/Checking/CheckRecordSyntaxHelpers.fs

@@ -33,7 +33,7 @@ let GroupUpdatesToNestedFields (fields: ((Ident list * Ident) * SynExpr option)
     let rec groupIfNested res xs =
         match xs with
         | [] -> res
-        | x :: [] -> x :: res
+        | [ x ] -> x :: res
         | x :: y :: ys ->
             match x, y with
             | (lidwid, Some(SynExpr.Record(baseInfo, copyInfo, fields1, m))), (_, Some(SynExpr.Record(recordFields = fields2))) ->

src/Compiler/Checking/ConstraintSolver.fs

@@ -1066,7 +1066,7 @@ and SolveAnonInfoEqualsAnonInfo (csenv: ConstraintSolverEnv) m2 (anonInfo1: Anon
             if not (ccuEq anonInfo1.Assembly anonInfo2.Assembly) then
                 do! ErrorD (ConstraintSolverError(FSComp.SR.tcAnonRecdCcuMismatch(anonInfo1.Assembly.AssemblyName, anonInfo2.Assembly.AssemblyName), csenv.m,m2))
 
-            if not (anonInfo1.SortedNames = anonInfo2.SortedNames) then 
+            if anonInfo1.SortedNames <> anonInfo2.SortedNames then 
                 let (|Subset|Superset|Overlap|CompletelyDifferent|) (first, second) =
                     let first = Set first
                     let second = Set second

src/Compiler/Checking/MethodOverrides.fs

@@ -439,10 +439,7 @@ module DispatchSlotChecking =
                 // check any of the missing overrides has isReqdTyInterface flag set
                 // in which case we use the message "with suggestion"
                 missingOverloadImplementation 
-                |> Seq.map fst 
-                |> Seq.filter id 
-                |> Seq.isEmpty
-                |> not
+                |> Seq.exists fst 
 
             if missingOverloadImplementation.Count = 1 then
                 // only one missing override, we have specific message for that
@@ -455,8 +452,7 @@ module DispatchSlotChecking =
                 let signatures = 
                     (missingOverloadImplementation 
                     |> Seq.truncate maxDisplayedOverrides 
-                    |> Seq.map snd 
-                    |> Seq.map (fun signature -> System.Environment.NewLine + "\t'" + signature.Value + "'")
+                    |> Seq.map (snd >> fun signature -> System.Environment.NewLine + "\t'" + signature.Value + "'")
                     |> String.concat "") + System.Environment.NewLine 
 
                 // we have specific message if the list is truncated
@@ -757,7 +753,7 @@ module DispatchSlotChecking =
                 GetIntrinsicPropInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes IgnoreOverrides reqdTyRange reqdTy 
                 |> List.filter isRelevantRequiredProperty
 
-            let dispatchSlots = dispatchSlotSet |> List.map snd |> List.concat
+            let dispatchSlots = dispatchSlotSet |> List.collect snd
             let dispatchSlotsKeyed = dispatchSlots |> NameMultiMap.initBy (fun reqdSlot -> reqdSlot.MethodInfo.LogicalName) 
             yield SlotImplSet(dispatchSlots, dispatchSlotsKeyed, availPriorOverrides, reqdProperties) ]