Fixing a few issues

dotnet · Sep 2, 2018 · 8da39ac · 8da39ac
1 parent c21a1d4
commit 8da39ac
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 14 deletions.
diff --git a/src/fsharp/Optimizer.fs b/src/fsharp/Optimizer.fs
@@ -1664,12 +1664,10 @@ let TryDetectQueryQuoteAndRun cenv (expr:Expr) =
         None
 
 let IsSystemStringConcat (methRef: ILMethodRef) =
-    let isConcat =
-        methRef.Name = "Concat" && methRef.DeclaringTypeRef.FullName = "System.String" && 
-        (methRef.ArgTypes |> List.forall(fun ilty -> ilty.BasicQualifiedName = "System.String") || 
-         (methRef.ArgTypes.Length = 1 && methRef.ArgTypes.Head.BasicQualifiedName = "System.String[]")) &&
-        methRef.ReturnType.BasicQualifiedName = "System.String"
-    isConcat
+    methRef.Name = "Concat" && methRef.DeclaringTypeRef.FullName = "System.String" && 
+    (methRef.ArgTypes |> List.forall(fun ilty -> ilty.BasicQualifiedName = "System.String") || 
+        (methRef.ArgTypes.Length = 1 && methRef.ArgTypes.Head.BasicQualifiedName = "System.String[]")) &&
+    methRef.ReturnType.BasicQualifiedName = "System.String"
 
 //-------------------------------------------------------------------------
 // The traversal
@@ -1832,6 +1830,7 @@ and OptimizeExprOp cenv env (op, tyargs, args, m) =
     // if the types match up. 
     | TOp.ILAsm([], [ty]), _, [a] when typeEquiv cenv.g (tyOfExpr cenv.g a) ty -> OptimizeExpr cenv env a
 
+    // Optimize calls when concatenating strings, e.g. "1" + "2" + "3" + "4" .. etc.
     | TOp.ILCall(_, _, _, _, _, _, _, methRef, _, _, _), _, opargs when IsSystemStringConcat methRef ->
 
         let rec flattenArgs e args =
@@ -1846,6 +1845,7 @@ and OptimizeExprOp cenv env (op, tyargs, args, m) =
                 (opargs, args)
                 ||> List.foldBack (fun arg args -> flattenArgs arg args)
 
+            // Optimize string constants, e.g. "1" + "2" will turn into "12"
             | Expr.Const(Const.String str1, _, _), Expr.Const(Const.String str2, _, _) :: args ->
                 mkString cenv.g m (str1 + str2) :: args
 
@@ -1863,23 +1863,27 @@ and OptimizeExprOp cenv env (op, tyargs, args, m) =
         let expr' =
             match args with
             | [ arg ] ->
-                arg
+                // If we only have one arg, check to see if it's an array. If it is then we are calling String.Concat(System.String[]).
+                if isArray1DTy cenv.g (tyOfExpr cenv.g arg) then
+                    mkStaticCall_String_Concat_Array cenv.g m arg
+                else
+                    arg
             | [ arg1; arg2 ] -> 
                 mkStaticCall_String_Concat2 cenv.g m arg1 arg2
             | [ arg1; arg2; arg3 ] ->
                 mkStaticCall_String_Concat3 cenv.g m arg1 arg2 arg3
             | [ arg1; arg2; arg3; arg4 ] ->
                 mkStaticCall_String_Concat4 cenv.g m arg1 arg2 arg3 arg4
             | args ->
-                mkStaticCall_String_Concat_Array cenv.g m args
+                let arg = mkArray (cenv.g.string_ty, args, m)
+                mkStaticCall_String_Concat_Array cenv.g m arg
 
         match expr' with
-        | Expr.Op(op', tyargs', args', m') when args.Length > 1 ->
+        | Expr.Op(TOp.ILCall(_, _, _, _, _, _, _, methRef, _, _, _) as op', tyargs', args', m') when IsSystemStringConcat methRef ->
             let args'', arginfos'' = OptimizeExprsThenConsiderSplits cenv env args'
             OptimizeExprOpFallback cenv env (op', tyargs', args'', m') arginfos'' UnknownValue
         | _ ->
             OptimizeExpr cenv env expr'
-
 
     | _ -> 
     (* Reductions *)

diff --git a/src/fsharp/TastOps.fs b/src/fsharp/TastOps.fs
@@ -6561,10 +6561,9 @@ let mkStaticCall_String_Concat4 g m arg1 arg2 arg3 arg4 =
     let mspec = mspec_String_Concat4 g
     Expr.Op(TOp.ILCall(false, false, false, false, ValUseFlag.NormalValUse, false, false, mspec.MethodRef, [], [], [g.string_ty]), [], [arg1; arg2; arg3; arg4], m)
 
-let mkStaticCall_String_Concat_Array g m args =
+let mkStaticCall_String_Concat_Array g m arg =
     let mspec = mspec_String_Concat_Array g
-    let e = mkArray (g.string_ty, args, m)
-    Expr.Op(TOp.ILCall(false, false, false, false, ValUseFlag.NormalValUse, false, false, mspec.MethodRef, [], [], [g.string_ty]), [], [e], m)
+    Expr.Op(TOp.ILCall(false, false, false, false, ValUseFlag.NormalValUse, false, false, mspec.MethodRef, [], [], [g.string_ty]), [], [arg], m)
 
 // Quotations can't contain any IL.
 // As a result, we aim to get rid of all IL generation in the typechecker and pattern match

diff --git a/src/fsharp/TastOps.fsi b/src/fsharp/TastOps.fsi
@@ -1378,7 +1378,7 @@ val mkArray : TType * Exprs * range -> Expr
 val mkStaticCall_String_Concat2 : TcGlobals -> range -> Expr -> Expr -> Expr
 val mkStaticCall_String_Concat3 : TcGlobals -> range -> Expr -> Expr -> Expr -> Expr
 val mkStaticCall_String_Concat4 : TcGlobals -> range -> Expr -> Expr -> Expr -> Expr -> Expr
-val mkStaticCall_String_Concat_Array : TcGlobals -> range -> Exprs -> Expr
+val mkStaticCall_String_Concat_Array : TcGlobals -> range -> Expr -> Expr
 
 //-------------------------------------------------------------------------
 // operations primarily associated with the optimization to fix