From 191950d00f779b51ffeeec89b21d183fc8328cd9 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 16 Jan 2024 16:33:21 +0100 Subject: [PATCH 01/20] Add unit tests for interp strings with string expr Sanity check that lowering to concat does not break these simple cases --- .../Language/InterpolatedStringsTests.fs | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs index c8f2eafa937..d0f7b316e36 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs @@ -128,4 +128,32 @@ type Foo () = x """ |> compile - |> shouldSucceed \ No newline at end of file + |> shouldSucceed + + [] + // Test different number of interpolated string parts + [] + [] + [] + let ``Interpolated expressions are strings`` (strToPrint: string, expectedOut: string) = + Fsx $""" +let x = {strToPrint} +printfn "%%s" x + """ + |> compileExeAndRun + |> shouldSucceed + |> withStdOutContains expectedOut + + [] + [] + [] + [] + let ``In FormattableString, interpolated expressions are strings`` (formattableStr: string, expectedOut: string, argCount: int) = + Fsx $""" +let x = {formattableStr} : System.FormattableString +assert(x.ArgumentCount = {argCount}) +printfn "%%s" (System.Globalization.CultureInfo "en-US" |> x.ToString) + """ + |> compileExeAndRun + |> shouldSucceed + |> withStdOutContains expectedOut \ No newline at end of file From 0bb23238d8fbe955475bd69b59dae2ef5f1419f4 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 16 Jan 2024 18:41:50 +0100 Subject: [PATCH 02/20] Add IL test for lowering to concat --- .../EmittedIL/StringFormatAndInterpolation.fs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs index 0ef7d23422e..4a0e2c6062d 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs @@ -5,8 +5,8 @@ namespace EmittedIL open Xunit open FSharp.Test.Compiler -#if !DEBUG // sensitive to debug-level code coming across from debug FSharp.Core module ``StringFormatAndInterpolation`` = +#if !DEBUG // sensitive to debug-level code coming across from debug FSharp.Core [] let ``Interpolated string with no holes is reduced to a string or simple format when used in printf``() = FSharp """ @@ -34,3 +34,21 @@ IL_0017: ret"""] #endif + [] + let ``Interpolated string with 3 parts consisting only of strings is lowered to concat`` () = + let str = "$\"\"\"ab{\"c\"}d\"\"\"" + FSharp $""" +module StringFormatAndInterpolation + +let str () = {str} + """ + |> compile + |> shouldSucceed + |> verifyIL [""" +IL_0000: ldstr "ab" +IL_0005: ldstr "c" +IL_000a: ldstr "d" +IL_000f: call string [runtime]System.String::Concat(string, + string, + string) +IL_0014: ret"""] \ No newline at end of file From 6d67b56a7b87617f9d39f8fdf1635c1f8a52f195 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 16 Jan 2024 15:20:34 +0100 Subject: [PATCH 03/20] Add optimization in CheckExpressions Initial attempt with many TODOs, also not sure whether it should be done in checking, but it seems that later we would have to again parse the string (since CheckExpressions is going from AST version of an interpolated string to a sprintf call basically) --- src/Compiler/Checking/CheckExpressions.fs | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index e59fb6bd5ff..b095a14d49b 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -7336,6 +7336,33 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn // Type check the expressions filling the holes let fillExprs, tpenv = TcExprsNoFlexes cenv env m tpenv argTys synFillExprs + // If all fill expressions are strings and there is less then 5 parts of the interpolated string total + // then we can use System.String.Concat instead of a sprintf call + if isString && (parts.Length < 5) && (argTys |> List.forall (isStringTy g)) then + let rec f xs ys acc = + match xs with + | SynInterpolatedStringPart.String(s, m)::xs -> + let sb = System.Text.StringBuilder(s).Replace("%%", "%") + f xs ys ((mkString g m (sb.ToString()))::acc) + | SynInterpolatedStringPart.FillExpr(_, _)::xs -> + match ys with + | y::ys -> f xs ys (y::acc) + | _ -> error(Error((0, "FOOBAR"), m)) // TODO XXX wrong error + | _ -> acc + + let args = f parts fillExprs [] |> List.rev + assert (args.Length = parts.Length) + if args.Length = 4 then + (mkStaticCall_String_Concat4 g m args[0] args[1] args[2] args[3], tpenv) + elif args.Length = 3 then + (mkStaticCall_String_Concat3 g m args[0] args[1] args[2], tpenv) + elif args.Length = 2 then + (mkStaticCall_String_Concat2 g m args[0] args[1], tpenv) + else + // Throw some error + error(Error((0, "FOOBAR2"), m)) // TODO XXX wrong error + else // TODO XXX messed up indentation below + let fillExprsBoxed = (argTys, fillExprs) ||> List.map2 (mkCallBox g m) let argsExpr = mkArray (g.obj_ty, fillExprsBoxed, m) From 1804afe3bc92a6f9d7b588339c54fafe93e412d9 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 30 Jan 2024 19:07:02 +0100 Subject: [PATCH 04/20] Do not optimize when format specifiers are there Cannot really optimize this way if width and other flags are specified. Typed interpolated expressions should be possible to support, but skipping them for now (TODO). --- src/Compiler/Checking/CheckExpressions.fs | 11 ++++++--- src/Compiler/Checking/CheckFormatStrings.fs | 26 ++++++++------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index b095a14d49b..2c11b9556b2 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -7189,6 +7189,7 @@ and TcFormatStringExpr cenv (overallTy: OverallTy) env m tpenv (fmtString: strin and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: SynInterpolatedStringPart list) = let g = cenv.g + let mutable allSimpleFormats = true let synFillExprs = parts |> List.choose (function @@ -7196,7 +7197,9 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn | SynInterpolatedStringPart.FillExpr (fillExpr, _) -> match fillExpr with // Detect "x" part of "...{x,3}..." - | SynExpr.Tuple (false, [e; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> Some e + | SynExpr.Tuple (false, [e; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> + allSimpleFormats <- false + Some e | e -> Some e) let stringFragmentRanges = @@ -7302,12 +7305,14 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn () | _ -> () - let argTys, _printerTy, printerTupleTyRequired, percentATys, _specifierLocations, dotnetFormatString = + let argTys, _printerTy, printerTupleTyRequired, percentATys, specifierLocations, dotnetFormatString = try CheckFormatStrings.ParseFormatString m stringFragmentRanges g true isFormattableString None printfFormatString printerArgTy printerResidueTy printerResultTy with Failure errString -> error (Error(FSComp.SR.tcUnableToParseInterpolatedString errString, m)) + allSimpleFormats <- allSimpleFormats && specifierLocations.IsEmpty + // Check the expressions filling the holes if argTys.Length <> synFillExprs.Length then error (Error(FSComp.SR.tcInterpolationMixedWithPercent(), m)) @@ -7338,7 +7343,7 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn // If all fill expressions are strings and there is less then 5 parts of the interpolated string total // then we can use System.String.Concat instead of a sprintf call - if isString && (parts.Length < 5) && (argTys |> List.forall (isStringTy g)) then + if allSimpleFormats && isString && (parts.Length < 5) && (argTys |> List.forall (isStringTy g)) then let rec f xs ys acc = match xs with | SynInterpolatedStringPart.String(s, m)::xs -> diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index ec8b4ae2b91..3f50ba2a5a0 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -317,13 +317,10 @@ let parseFormatStringInternal |> Seq.takeWhile (fun c -> c = '%') |> Seq.length if delimLen <= 1 && fmt[i..(i+1)] = "%%" then - match context with - | Some _ -> - specifierLocations.Add( - (Range.mkFileIndexRange m.FileIndex - (Position.mkPos fragLine fragCol) - (Position.mkPos fragLine (fragCol+2))), 0) - | None -> () + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine fragCol) + (Position.mkPos fragLine (fragCol+2))), 0) appendToDotnetFormatString "%" parseLoop acc (i+2, fragLine, fragCol+2) fragments elif delimLen > 1 && nPercentSigns < delimLen then @@ -384,15 +381,12 @@ let parseFormatStringInternal failwith (FSComp.SR.forFormatInvalidForInterpolated3()) let collectSpecifierLocation fragLine fragCol numStdArgs = - match context with - | Some _ -> - let numArgsForSpecifier = - numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) - specifierLocations.Add( - (Range.mkFileIndexRange m.FileIndex - (Position.mkPos fragLine startFragCol) - (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) - | None -> () + let numArgsForSpecifier = + numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine startFragCol) + (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) let ch = fmt[i] match ch with From 12e231eab8d5333f67a260d71027f9853d4ef6f4 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 30 Jan 2024 21:14:59 +0100 Subject: [PATCH 05/20] Adjust expected length of codegen --- tests/AheadOfTime/Trimming/check.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/AheadOfTime/Trimming/check.ps1 b/tests/AheadOfTime/Trimming/check.ps1 index 4b9dcaebf16..8528710b76f 100644 --- a/tests/AheadOfTime/Trimming/check.ps1 +++ b/tests/AheadOfTime/Trimming/check.ps1 @@ -42,4 +42,4 @@ function CheckTrim($root, $tfm, $outputfile, $expected_len) { CheckTrim -root "SelfContained_Trimming_Test" -tfm "net8.0" -outputfile "FSharp.Core.dll" -expected_len 287232 # Check net7.0 trimmed assemblies -CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net8.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 8821248 +CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net8.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 8820736 From 7b7b4c70a4d9194baf96e3cf7f04e0a84eac4c03 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 2 Feb 2024 14:19:10 +0100 Subject: [PATCH 06/20] Filter out empty string parts E.g. $"{x}{y}" has 5 string parts, including 3 empty strings --- src/Compiler/Checking/CheckExpressions.fs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 2c11b9556b2..3c361412e94 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -7341,9 +7341,16 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn // Type check the expressions filling the holes let fillExprs, tpenv = TcExprsNoFlexes cenv env m tpenv argTys synFillExprs + let nonEmptyParts = + parts + |> List.filter (fun x -> + match x with + | SynInterpolatedStringPart.String(s, _) -> not <| System.String.IsNullOrEmpty s + | SynInterpolatedStringPart.FillExpr(_, _) -> true) + // If all fill expressions are strings and there is less then 5 parts of the interpolated string total // then we can use System.String.Concat instead of a sprintf call - if allSimpleFormats && isString && (parts.Length < 5) && (argTys |> List.forall (isStringTy g)) then + if allSimpleFormats && isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) then let rec f xs ys acc = match xs with | SynInterpolatedStringPart.String(s, m)::xs -> @@ -7355,14 +7362,16 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn | _ -> error(Error((0, "FOOBAR"), m)) // TODO XXX wrong error | _ -> acc - let args = f parts fillExprs [] |> List.rev - assert (args.Length = parts.Length) + let args = f nonEmptyParts fillExprs [] |> List.rev + assert (args.Length = nonEmptyParts.Length) if args.Length = 4 then (mkStaticCall_String_Concat4 g m args[0] args[1] args[2] args[3], tpenv) elif args.Length = 3 then (mkStaticCall_String_Concat3 g m args[0] args[1] args[2], tpenv) elif args.Length = 2 then (mkStaticCall_String_Concat2 g m args[0] args[1], tpenv) + elif args.Length = 1 then + args[0], tpenv else // Throw some error error(Error((0, "FOOBAR2"), m)) // TODO XXX wrong error From 03200aec46f6ae4a600498ef136b173f16f0e04c Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 2 Feb 2024 16:21:22 +0100 Subject: [PATCH 07/20] Detect format specifiers better There were false positives before --- src/Compiler/Checking/CheckExpressions.fs | 47 +++++++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 3c361412e94..39548db37d0 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -140,6 +140,27 @@ exception StandardOperatorRedefinitionWarning of string * range exception InvalidInternalsVisibleToAssemblyName of badName: string * fileName: string option +// In other parts of the codebase, when we crack open strings, we do +// manual sort of parsing. I feel like this is actually more readable, +// and it should be enough since we are only interested in format specifiers +// TODO XXX +module private ParseString = + // Regex pattern for something like: %[flags][width][.precision][type] + // but match only if % is not escaped by another % + let formatSpecifier = + let sb = System.Text.StringBuilder(64) + sb.Append(@"(^|[^%])") |> ignore // Start with beginning of string or any char other than '%' + sb.Append(@"(%%)*%") |> ignore // followed by an odd number of '%' chars + sb.Append(@"[+-0 ]?") |> ignore // optionally followed by flags + sb.Append(@"(\d+)?") |> ignore // optionally followed by width + sb.Append(@"(\.\d+)?") |> ignore // optionally followed by .precision + sb.Append(@"[bscdiuxXoBeEfFgGMOAat]") |> ignore // and then a char that determines specifier's type + let pattern = sb.ToString() + System.Text.RegularExpressions.Regex( + pattern, + System.Text.RegularExpressions.RegexOptions.Compiled) + let HasFormatSpecifier = formatSpecifier.IsMatch + /// Compute the available access rights from a particular location in code let ComputeAccessRights eAccessPath eInternalsVisibleCompPaths eFamilyType = AccessibleFrom (eAccessPath :: eInternalsVisibleCompPaths, eFamilyType) @@ -7189,7 +7210,6 @@ and TcFormatStringExpr cenv (overallTy: OverallTy) env m tpenv (fmtString: strin and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: SynInterpolatedStringPart list) = let g = cenv.g - let mutable allSimpleFormats = true let synFillExprs = parts |> List.choose (function @@ -7197,9 +7217,7 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn | SynInterpolatedStringPart.FillExpr (fillExpr, _) -> match fillExpr with // Detect "x" part of "...{x,3}..." - | SynExpr.Tuple (false, [e; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> - allSimpleFormats <- false - Some e + | SynExpr.Tuple (false, [e; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> Some e | e -> Some e) let stringFragmentRanges = @@ -7305,14 +7323,12 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn () | _ -> () - let argTys, _printerTy, printerTupleTyRequired, percentATys, specifierLocations, dotnetFormatString = + let argTys, _printerTy, printerTupleTyRequired, percentATys, _specifierLocations, dotnetFormatString = try CheckFormatStrings.ParseFormatString m stringFragmentRanges g true isFormattableString None printfFormatString printerArgTy printerResidueTy printerResultTy with Failure errString -> error (Error(FSComp.SR.tcUnableToParseInterpolatedString errString, m)) - allSimpleFormats <- allSimpleFormats && specifierLocations.IsEmpty - // Check the expressions filling the holes if argTys.Length <> synFillExprs.Length then error (Error(FSComp.SR.tcInterpolationMixedWithPercent(), m)) @@ -7348,9 +7364,24 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn | SynInterpolatedStringPart.String(s, _) -> not <| System.String.IsNullOrEmpty s | SynInterpolatedStringPart.FillExpr(_, _) -> true) + // TODO XXX If there is a format specifer that only specifies type + // of the interpolated expression (e.g. $"%s{x}"), we can just + // erase if from the string and carry on? + let noSpecifiers = + nonEmptyParts + |> List.forall (fun x -> + match x with + | SynInterpolatedStringPart.FillExpr(expr, _) -> + match expr with + // Detect "x" part of "...{x,3}..." + // TODO XXX should also detect {x:Y} probably + | SynExpr.Tuple (false, [_; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> false + | _ -> true + | SynInterpolatedStringPart.String(s, _) -> not <| ParseString.HasFormatSpecifier s) + // If all fill expressions are strings and there is less then 5 parts of the interpolated string total // then we can use System.String.Concat instead of a sprintf call - if allSimpleFormats && isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) then + if noSpecifiers && isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) then let rec f xs ys acc = match xs with | SynInterpolatedStringPart.String(s, m)::xs -> From 9d7419d19f2e8e15fea9787681ebfb0bf78aac19 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 6 Feb 2024 18:47:34 +0100 Subject: [PATCH 08/20] Refactor, improve regex --- src/Compiler/Checking/CheckExpressions.fs | 118 ++++++++++++---------- 1 file changed, 66 insertions(+), 52 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 39548db37d0..f7d4f50c0c7 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -140,25 +140,29 @@ exception StandardOperatorRedefinitionWarning of string * range exception InvalidInternalsVisibleToAssemblyName of badName: string * fileName: string option -// In other parts of the codebase, when we crack open strings, we do -// manual sort of parsing. I feel like this is actually more readable, -// and it should be enough since we are only interested in format specifiers -// TODO XXX +/// A set of helpers for determining if/what specifiers a string has. +/// Used to decide if interpolated string can be lowered to a concat call. +/// We don't have to care about single- vs multi-$ strings here, because lexer took care of that already. module private ParseString = + open System.Text.RegularExpressions + + // Regex for strings ending with unescaped '%s' + // For efficiency sake using lookbehind, starting from end of the string + let stringSpecifier = Regex(@"$(?<=(^|[^%])(%%)*%s)", RegexOptions.Compiled) + let EndsWithStringSpecifierNoFlags = stringSpecifier.IsMatch + // Regex pattern for something like: %[flags][width][.precision][type] // but match only if % is not escaped by another % let formatSpecifier = - let sb = System.Text.StringBuilder(64) + let sb = Text.StringBuilder(64) sb.Append(@"(^|[^%])") |> ignore // Start with beginning of string or any char other than '%' sb.Append(@"(%%)*%") |> ignore // followed by an odd number of '%' chars - sb.Append(@"[+-0 ]?") |> ignore // optionally followed by flags + sb.Append(@"[+-0 ]{0,3}") |> ignore // optionally followed by flags sb.Append(@"(\d+)?") |> ignore // optionally followed by width sb.Append(@"(\.\d+)?") |> ignore // optionally followed by .precision sb.Append(@"[bscdiuxXoBeEfFgGMOAat]") |> ignore // and then a char that determines specifier's type let pattern = sb.ToString() - System.Text.RegularExpressions.Regex( - pattern, - System.Text.RegularExpressions.RegexOptions.Compiled) + Regex(pattern, RegexOptions.Compiled) let HasFormatSpecifier = formatSpecifier.IsMatch /// Compute the available access rights from a particular location in code @@ -7357,43 +7361,54 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn // Type check the expressions filling the holes let fillExprs, tpenv = TcExprsNoFlexes cenv env m tpenv argTys synFillExprs + // If all fill expressions are strings and there is less then 5 parts of the interpolated string total + // then we can use System.String.Concat instead of a sprintf call. + // First filter out all string parts that are empty or only have a '%s' string specifier, + // also get rid of the '%s' specifier right before the interpolation expression. let nonEmptyParts = parts + |> List.map (fun x -> + match x with + | SynInterpolatedStringPart.String(s, m) when ParseString.EndsWithStringSpecifierNoFlags s -> + SynInterpolatedStringPart.String(s.Substring(0, s.Length-2), shiftEnd 0 (-2) m) + | _ -> x) |> List.filter (fun x -> match x with | SynInterpolatedStringPart.String(s, _) -> not <| System.String.IsNullOrEmpty s | SynInterpolatedStringPart.FillExpr(_, _) -> true) - // TODO XXX If there is a format specifer that only specifies type - // of the interpolated expression (e.g. $"%s{x}"), we can just - // erase if from the string and carry on? - let noSpecifiers = - nonEmptyParts + // Then check if there are any format specifiers that might require actual formatting (as opposed to type checking) + let noSpecifiers stringParts = + stringParts |> List.forall (fun x -> match x with - | SynInterpolatedStringPart.FillExpr(expr, _) -> - match expr with + | SynInterpolatedStringPart.FillExpr(expr, qualifiers) -> + match expr, qualifiers with + // Something like "...{x:hh}..." + | _, Some _ -> false // Detect "x" part of "...{x,3}..." - // TODO XXX should also detect {x:Y} probably - | SynExpr.Tuple (false, [_; SynExpr.Const (SynConst.Int32 _align, _)], _, _) -> false + | SynExpr.Tuple (false, [_; SynExpr.Const (SynConst.Int32 _align, _)], _, _), _ -> false | _ -> true | SynInterpolatedStringPart.String(s, _) -> not <| ParseString.HasFormatSpecifier s) - // If all fill expressions are strings and there is less then 5 parts of the interpolated string total - // then we can use System.String.Concat instead of a sprintf call - if noSpecifiers && isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) then - let rec f xs ys acc = - match xs with - | SynInterpolatedStringPart.String(s, m)::xs -> - let sb = System.Text.StringBuilder(s).Replace("%%", "%") - f xs ys ((mkString g m (sb.ToString()))::acc) - | SynInterpolatedStringPart.FillExpr(_, _)::xs -> - match ys with - | y::ys -> f xs ys (y::acc) - | _ -> error(Error((0, "FOOBAR"), m)) // TODO XXX wrong error - | _ -> acc - - let args = f nonEmptyParts fillExprs [] |> List.rev + if isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) && (noSpecifiers nonEmptyParts) then + + let args = + // We already have the type-checked fill expressions from before + // so let's just weave them with the string parts in the right order + nonEmptyParts + |> List.fold (fun (fillExprs, acc) stringPart -> + match stringPart with + | SynInterpolatedStringPart.String(s, m) -> + let sb = System.Text.StringBuilder(s).Replace("%%", "%") + (fillExprs, (mkString g m (sb.ToString()))::acc) + | SynInterpolatedStringPart.FillExpr(_, _) -> + match fillExprs with + | expr::exprs -> (exprs, expr::acc) + | _ -> error(InternalError("Mismatch in interpolation expression count", m))) (fillExprs, []) + |> snd + |> List.rev + assert (args.Length = nonEmptyParts.Length) if args.Length = 4 then (mkStaticCall_String_Concat4 g m args[0] args[1] args[2] args[3], tpenv) @@ -7404,29 +7419,28 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn elif args.Length = 1 then args[0], tpenv else - // Throw some error - error(Error((0, "FOOBAR2"), m)) // TODO XXX wrong error - else // TODO XXX messed up indentation below + error(InternalError("Mismatch in arg count when lowering to Concat", m)) + else - let fillExprsBoxed = (argTys, fillExprs) ||> List.map2 (mkCallBox g m) + let fillExprsBoxed = (argTys, fillExprs) ||> List.map2 (mkCallBox g m) - let argsExpr = mkArray (g.obj_ty, fillExprsBoxed, m) - let percentATysExpr = - if percentATys.Length = 0 then - mkNull m (mkArrayType g g.system_Type_ty) - else - let tyExprs = percentATys |> Array.map (mkCallTypeOf g m) |> Array.toList - mkArray (g.system_Type_ty, tyExprs, m) + let argsExpr = mkArray (g.obj_ty, fillExprsBoxed, m) + let percentATysExpr = + if percentATys.Length = 0 then + mkNull m (mkArrayType g g.system_Type_ty) + else + let tyExprs = percentATys |> Array.map (mkCallTypeOf g m) |> Array.toList + mkArray (g.system_Type_ty, tyExprs, m) - let fmtExpr = MakeMethInfoCall cenv.amap m newFormatMethod [] [mkString g m printfFormatString; argsExpr; percentATysExpr] None + let fmtExpr = MakeMethInfoCall cenv.amap m newFormatMethod [] [mkString g m printfFormatString; argsExpr; percentATysExpr] None - if isString then - TcPropagatingExprLeafThenConvert cenv overallTy g.string_ty env (* true *) m (fun () -> - // Make the call to sprintf - mkCall_sprintf g m printerTy fmtExpr [], tpenv - ) - else - fmtExpr, tpenv + if isString then + TcPropagatingExprLeafThenConvert cenv overallTy g.string_ty env (* true *) m (fun () -> + // Make the call to sprintf + mkCall_sprintf g m printerTy fmtExpr [], tpenv + ) + else + fmtExpr, tpenv // The case for $"..." used as type FormattableString or IFormattable | Choice2Of2 createFormattableStringMethod -> From 8f3d2b22ee786b8aabc681b9dd91795cc760c6db Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 6 Feb 2024 18:48:28 +0100 Subject: [PATCH 09/20] Unrelated indentation fix in parser --- src/Compiler/pars.fsy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy index 6c5415667d0..4080fc7ec8b 100644 --- a/src/Compiler/pars.fsy +++ b/src/Compiler/pars.fsy @@ -6776,8 +6776,8 @@ interpolatedStringParts: | INTERP_STRING_END { [ SynInterpolatedStringPart.String(fst $1, rhs parseState 1) ] } - | INTERP_STRING_PART interpolatedStringFill interpolatedStringParts - { SynInterpolatedStringPart.String(fst $1, rhs parseState 1) :: SynInterpolatedStringPart.FillExpr $2 :: $3 } + | INTERP_STRING_PART interpolatedStringFill interpolatedStringParts + { SynInterpolatedStringPart.String(fst $1, rhs parseState 1) :: SynInterpolatedStringPart.FillExpr $2 :: $3 } | INTERP_STRING_PART interpolatedStringParts { let rbrace = parseState.InputEndPosition 1 From af418a3b08b691ccdc569c6d8cd5455991bf53e9 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 14 Feb 2024 18:17:04 +0100 Subject: [PATCH 10/20] Use language feature flag --- src/Compiler/Checking/CheckExpressions.fs | 16 ++++++++++++---- src/Compiler/FSComp.txt | 1 + src/Compiler/Facilities/LanguageFeatures.fs | 3 +++ src/Compiler/Facilities/LanguageFeatures.fsi | 1 + 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index f7d4f50c0c7..fc3855085ea 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -140,9 +140,11 @@ exception StandardOperatorRedefinitionWarning of string * range exception InvalidInternalsVisibleToAssemblyName of badName: string * fileName: string option -/// A set of helpers for determining if/what specifiers a string has. -/// Used to decide if interpolated string can be lowered to a concat call. -/// We don't have to care about single- vs multi-$ strings here, because lexer took care of that already. +//---------------------------------------------------------------------------------------------- +// Helpers for determining if/what specifiers a string has. +// Used to decide if interpolated string can be lowered to a concat call. +// We don't care about single- vs multi-$ strings here, because lexer took care of that already. +//---------------------------------------------------------------------------------------------- module private ParseString = open System.Text.RegularExpressions @@ -7391,7 +7393,13 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn | _ -> true | SynInterpolatedStringPart.String(s, _) -> not <| ParseString.HasFormatSpecifier s) - if isString && (nonEmptyParts.Length < 5) && (argTys |> List.forall (isStringTy g)) && (noSpecifiers nonEmptyParts) then + if + (g.langVersion.SupportsFeature LanguageFeature.LowerInterpolatedStringToConcat) + && isString + && (nonEmptyParts.Length < 5) + && (argTys |> List.forall (isStringTy g)) + && (noSpecifiers nonEmptyParts) + then let args = // We already have the type-checked fill expressions from before diff --git a/src/Compiler/FSComp.txt b/src/Compiler/FSComp.txt index a9fc2692157..60601698bc0 100644 --- a/src/Compiler/FSComp.txt +++ b/src/Compiler/FSComp.txt @@ -1594,6 +1594,7 @@ featureWarningIndexedPropertiesGetSetSameType,"Indexed properties getter and set featureChkTailCallAttrOnNonRec,"Raises warnings if the 'TailCall' attribute is used on non-recursive functions." featureUnionIsPropertiesVisible,"Union case test properties" featureBooleanReturningAndReturnTypeDirectedPartialActivePattern,"Boolean-returning and return-type-directed partial active patterns" +featureLowerInterpolatedStringToConcat,"Optimizes interpolated strings in certains cases, by lowering to concatenation" 3354,tcNotAFunctionButIndexerNamedIndexingNotYetEnabled,"This value supports indexing, e.g. '%s.[index]'. The syntax '%s[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation." 3354,tcNotAFunctionButIndexerIndexingNotYetEnabled,"This expression supports indexing, e.g. 'expr.[index]'. The syntax 'expr[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation." 3355,tcNotAnIndexerNamedIndexingNotYetEnabled,"The value '%s' is not a function and does not support index notation." diff --git a/src/Compiler/Facilities/LanguageFeatures.fs b/src/Compiler/Facilities/LanguageFeatures.fs index e7c2a25ee3d..66547a12e29 100644 --- a/src/Compiler/Facilities/LanguageFeatures.fs +++ b/src/Compiler/Facilities/LanguageFeatures.fs @@ -85,6 +85,7 @@ type LanguageFeature = | WarningIndexedPropertiesGetSetSameType | WarningWhenTailCallAttrOnNonRec | BooleanReturningAndReturnTypeDirectedPartialActivePattern + | LowerInterpolatedStringToConcat /// LanguageVersion management type LanguageVersion(versionText) = @@ -197,6 +198,7 @@ type LanguageVersion(versionText) = LanguageFeature.WarningWhenTailCallAttrOnNonRec, previewVersion LanguageFeature.UnionIsPropertiesVisible, previewVersion LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern, previewVersion + LanguageFeature.LowerInterpolatedStringToConcat, previewVersion ] static let defaultLanguageVersion = LanguageVersion("default") @@ -340,6 +342,7 @@ type LanguageVersion(versionText) = | LanguageFeature.WarningWhenTailCallAttrOnNonRec -> FSComp.SR.featureChkTailCallAttrOnNonRec () | LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern -> FSComp.SR.featureBooleanReturningAndReturnTypeDirectedPartialActivePattern () + | LanguageFeature.LowerInterpolatedStringToConcat -> FSComp.SR.featureLowerInterpolatedStringToConcat () /// Get a version string associated with the given feature. static member GetFeatureVersionString feature = diff --git a/src/Compiler/Facilities/LanguageFeatures.fsi b/src/Compiler/Facilities/LanguageFeatures.fsi index 29d6c2c33a3..582c191ac8a 100644 --- a/src/Compiler/Facilities/LanguageFeatures.fsi +++ b/src/Compiler/Facilities/LanguageFeatures.fsi @@ -76,6 +76,7 @@ type LanguageFeature = | WarningIndexedPropertiesGetSetSameType | WarningWhenTailCallAttrOnNonRec | BooleanReturningAndReturnTypeDirectedPartialActivePattern + | LowerInterpolatedStringToConcat /// LanguageVersion management type LanguageVersion = From 467e8769a7ef76163ce2cb253f4eea30419f1279 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 15 Feb 2024 17:11:38 +0100 Subject: [PATCH 11/20] FSComp.txt auto-update --- src/Compiler/xlf/FSComp.txt.cs.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.de.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.es.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.fr.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.it.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.ja.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.ko.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.pl.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.pt-BR.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.ru.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.tr.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.zh-Hans.xlf | 5 +++++ src/Compiler/xlf/FSComp.txt.zh-Hant.xlf | 5 +++++ 13 files changed, 65 insertions(+) diff --git a/src/Compiler/xlf/FSComp.txt.cs.xlf b/src/Compiler/xlf/FSComp.txt.cs.xlf index c7dbc64bac7..47741eeb54a 100644 --- a/src/Compiler/xlf/FSComp.txt.cs.xlf +++ b/src/Compiler/xlf/FSComp.txt.cs.xlf @@ -372,6 +372,11 @@ rozhraní s vícenásobným obecným vytvářením instancí + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Povolit duplikát malými písmeny při atributu RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.de.xlf b/src/Compiler/xlf/FSComp.txt.de.xlf index d5800420f00..7f233df4bb1 100644 --- a/src/Compiler/xlf/FSComp.txt.de.xlf +++ b/src/Compiler/xlf/FSComp.txt.de.xlf @@ -372,6 +372,11 @@ Schnittstellen mit mehrfacher generischer Instanziierung + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute DU in Kleinbuchstaben zulassen, wenn requireQualifiedAccess-Attribut diff --git a/src/Compiler/xlf/FSComp.txt.es.xlf b/src/Compiler/xlf/FSComp.txt.es.xlf index dcfce175697..91912f4eda1 100644 --- a/src/Compiler/xlf/FSComp.txt.es.xlf +++ b/src/Compiler/xlf/FSComp.txt.es.xlf @@ -372,6 +372,11 @@ interfaces con creación de instancias genéricas múltiples + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Permitir DU en minúsculas con el atributo RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.fr.xlf b/src/Compiler/xlf/FSComp.txt.fr.xlf index 590c4fb8e15..d5f5133b094 100644 --- a/src/Compiler/xlf/FSComp.txt.fr.xlf +++ b/src/Compiler/xlf/FSComp.txt.fr.xlf @@ -372,6 +372,11 @@ interfaces avec plusieurs instanciations génériques + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Autoriser les DU en minuscules pour l'attribut RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.it.xlf b/src/Compiler/xlf/FSComp.txt.it.xlf index 70613984b76..2b1e8e62c7a 100644 --- a/src/Compiler/xlf/FSComp.txt.it.xlf +++ b/src/Compiler/xlf/FSComp.txt.it.xlf @@ -372,6 +372,11 @@ interfacce con più creazioni di istanze generiche + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Consentire l’unione discriminata minuscola quando l'attributo RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.ja.xlf b/src/Compiler/xlf/FSComp.txt.ja.xlf index 84a9ca6ef70..1a9e62e9a73 100644 --- a/src/Compiler/xlf/FSComp.txt.ja.xlf +++ b/src/Compiler/xlf/FSComp.txt.ja.xlf @@ -372,6 +372,11 @@ 複数のジェネリックのインスタンス化を含むインターフェイス + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute RequireQualifiedAccess 属性の場合に小文字 DU を許可する diff --git a/src/Compiler/xlf/FSComp.txt.ko.xlf b/src/Compiler/xlf/FSComp.txt.ko.xlf index 38ffac7d32d..0c67bd0dded 100644 --- a/src/Compiler/xlf/FSComp.txt.ko.xlf +++ b/src/Compiler/xlf/FSComp.txt.ko.xlf @@ -372,6 +372,11 @@ 여러 제네릭 인스턴스화가 포함된 인터페이스 + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute RequireQualifiedAccess 특성이 있는 경우 소문자 DU 허용 diff --git a/src/Compiler/xlf/FSComp.txt.pl.xlf b/src/Compiler/xlf/FSComp.txt.pl.xlf index ac1d54f63c4..61b93351bdd 100644 --- a/src/Compiler/xlf/FSComp.txt.pl.xlf +++ b/src/Compiler/xlf/FSComp.txt.pl.xlf @@ -372,6 +372,11 @@ interfejsy z wieloma ogólnymi wystąpieniami + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Zezwalaj na małą literę DU, gdy występuje RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.pt-BR.xlf b/src/Compiler/xlf/FSComp.txt.pt-BR.xlf index 6beb1946267..1b31443e280 100644 --- a/src/Compiler/xlf/FSComp.txt.pt-BR.xlf +++ b/src/Compiler/xlf/FSComp.txt.pt-BR.xlf @@ -372,6 +372,11 @@ interfaces com várias instanciações genéricas + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Permitir DU em minúsculas quando o atributo RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.ru.xlf b/src/Compiler/xlf/FSComp.txt.ru.xlf index 1b83dc7cc6b..a21158f8391 100644 --- a/src/Compiler/xlf/FSComp.txt.ru.xlf +++ b/src/Compiler/xlf/FSComp.txt.ru.xlf @@ -372,6 +372,11 @@ интерфейсы с множественным универсальным созданием экземпляра + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute Разрешить du в нижнем регистре, если атрибут RequireQualifiedAccess diff --git a/src/Compiler/xlf/FSComp.txt.tr.xlf b/src/Compiler/xlf/FSComp.txt.tr.xlf index 43f5069b2eb..ef179892d69 100644 --- a/src/Compiler/xlf/FSComp.txt.tr.xlf +++ b/src/Compiler/xlf/FSComp.txt.tr.xlf @@ -372,6 +372,11 @@ birden çok genel örnek oluşturma içeren arabirimler + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute RequireQualifiedAccess özniteliğinde küçük harf DU'ya izin ver diff --git a/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf b/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf index 91cad01e99e..bfbb2bd4593 100644 --- a/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf +++ b/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf @@ -372,6 +372,11 @@ 具有多个泛型实例化的接口 + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute 当 RequireQualifiedAccess 属性时允许小写 DU diff --git a/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf b/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf index f12b3fd0b5e..21cbc41d453 100644 --- a/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf +++ b/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf @@ -372,6 +372,11 @@ 具有多個泛型具現化的介面 + + Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certains cases, by lowering to concatenation + + Allow lowercase DU when RequireQualifiedAccess attribute RequireQualifiedAccess 屬性時允許小寫 DU From a60c558e5a25a21846ee1acf8989d7dca018e8ea Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 15 Feb 2024 17:18:10 +0100 Subject: [PATCH 12/20] Add release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + docs/release-notes/.Language/preview.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 672209d2ed1..59480b39a07 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -29,3 +29,4 @@ * Reduce allocations in compiler checking via `ValueOption` usage ([PR #16323](https://github.com/dotnet/fsharp/pull/16323), [PR #16567](https://github.com/dotnet/fsharp/pull/16567)) * Reverted [#16348](https://github.com/dotnet/fsharp/pull/16348) `ThreadStatic` `CancellationToken` changes to improve test stability and prevent potential unwanted cancellations. ([PR #16536](https://github.com/dotnet/fsharp/pull/16536)) * Refactored parenthesization API. ([PR #16461])(https://github.com/dotnet/fsharp/pull/16461)) +* Optimize some interpolated strings by lowering to string concatenation. ([PR #16556](https://github.com/dotnet/fsharp/pull/16556)) diff --git a/docs/release-notes/.Language/preview.md b/docs/release-notes/.Language/preview.md index 832dd924a44..d1880b4dd85 100644 --- a/docs/release-notes/.Language/preview.md +++ b/docs/release-notes/.Language/preview.md @@ -8,3 +8,7 @@ ### Fixed * Allow extension methods without type attribute work for types from imported assemblies. ([PR #16368](https://github.com/dotnet/fsharp/pull/16368)) + +### Changed + +* Lower interpolated strings to string concatenation. ([PR #16556](https://github.com/dotnet/fsharp/pull/16556)) \ No newline at end of file From d299d708339fe603cfa4096504d28a7b2bb136ce Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 16 Feb 2024 14:36:00 +0100 Subject: [PATCH 13/20] Add langversion flag to some unit tests --- .../EmittedIL/StringFormatAndInterpolation.fs | 1 + .../Language/InterpolatedStringsTests.fs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs index 4a0e2c6062d..1df9c914076 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs @@ -42,6 +42,7 @@ module StringFormatAndInterpolation let str () = {str} """ + |> withLangVersionPreview |> compile |> shouldSucceed |> verifyIL [""" diff --git a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs index d0f7b316e36..068deecc9e9 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs @@ -140,6 +140,7 @@ type Foo () = let x = {strToPrint} printfn "%%s" x """ + |> withLangVersionPreview |> compileExeAndRun |> shouldSucceed |> withStdOutContains expectedOut @@ -154,6 +155,7 @@ let x = {formattableStr} : System.FormattableString assert(x.ArgumentCount = {argCount}) printfn "%%s" (System.Globalization.CultureInfo "en-US" |> x.ToString) """ + |> withLangVersionPreview |> compileExeAndRun |> shouldSucceed |> withStdOutContains expectedOut \ No newline at end of file From f3c42b95307bba5789f3fdde9a3eafbe5bc0df85 Mon Sep 17 00:00:00 2001 From: Adam Boniecki <20281641+abonie@users.noreply.github.com> Date: Tue, 20 Feb 2024 13:16:01 +0100 Subject: [PATCH 14/20] Fix typo in src/Compiler/FSComp.txt Co-authored-by: Petr --- src/Compiler/FSComp.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/FSComp.txt b/src/Compiler/FSComp.txt index 60601698bc0..1fef46084d0 100644 --- a/src/Compiler/FSComp.txt +++ b/src/Compiler/FSComp.txt @@ -1594,7 +1594,7 @@ featureWarningIndexedPropertiesGetSetSameType,"Indexed properties getter and set featureChkTailCallAttrOnNonRec,"Raises warnings if the 'TailCall' attribute is used on non-recursive functions." featureUnionIsPropertiesVisible,"Union case test properties" featureBooleanReturningAndReturnTypeDirectedPartialActivePattern,"Boolean-returning and return-type-directed partial active patterns" -featureLowerInterpolatedStringToConcat,"Optimizes interpolated strings in certains cases, by lowering to concatenation" +featureLowerInterpolatedStringToConcat,"Optimizes interpolated strings in certain cases, by lowering to concatenation" 3354,tcNotAFunctionButIndexerNamedIndexingNotYetEnabled,"This value supports indexing, e.g. '%s.[index]'. The syntax '%s[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation." 3354,tcNotAFunctionButIndexerIndexingNotYetEnabled,"This expression supports indexing, e.g. 'expr.[index]'. The syntax 'expr[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation." 3355,tcNotAnIndexerNamedIndexingNotYetEnabled,"The value '%s' is not a function and does not support index notation." From 629812ef9c72d03beed9ade95011ed4b90ca8107 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 20 Feb 2024 14:17:19 +0100 Subject: [PATCH 15/20] Update .xlf and undo change to CheckFormatStrings --- src/Compiler/Checking/CheckFormatStrings.fs | 11 +++++++---- src/Compiler/xlf/FSComp.txt.cs.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.de.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.es.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.fr.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.it.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.ja.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.ko.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.pl.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.pt-BR.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.ru.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.tr.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.zh-Hans.xlf | 4 ++-- src/Compiler/xlf/FSComp.txt.zh-Hant.xlf | 4 ++-- 14 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index 3f50ba2a5a0..1d8523deb93 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -317,10 +317,13 @@ let parseFormatStringInternal |> Seq.takeWhile (fun c -> c = '%') |> Seq.length if delimLen <= 1 && fmt[i..(i+1)] = "%%" then - specifierLocations.Add( - (Range.mkFileIndexRange m.FileIndex - (Position.mkPos fragLine fragCol) - (Position.mkPos fragLine (fragCol+2))), 0) + match context with + | Some _ -> + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine fragCol) + (Position.mkPos fragLine (fragCol+2))), 0) + | None -> () appendToDotnetFormatString "%" parseLoop acc (i+2, fragLine, fragCol+2) fragments elif delimLen > 1 && nPercentSigns < delimLen then diff --git a/src/Compiler/xlf/FSComp.txt.cs.xlf b/src/Compiler/xlf/FSComp.txt.cs.xlf index 47741eeb54a..27098c6bd1e 100644 --- a/src/Compiler/xlf/FSComp.txt.cs.xlf +++ b/src/Compiler/xlf/FSComp.txt.cs.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.de.xlf b/src/Compiler/xlf/FSComp.txt.de.xlf index 7f233df4bb1..c89e0988f8e 100644 --- a/src/Compiler/xlf/FSComp.txt.de.xlf +++ b/src/Compiler/xlf/FSComp.txt.de.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.es.xlf b/src/Compiler/xlf/FSComp.txt.es.xlf index 91912f4eda1..de54c17c1f3 100644 --- a/src/Compiler/xlf/FSComp.txt.es.xlf +++ b/src/Compiler/xlf/FSComp.txt.es.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.fr.xlf b/src/Compiler/xlf/FSComp.txt.fr.xlf index d5f5133b094..17807523602 100644 --- a/src/Compiler/xlf/FSComp.txt.fr.xlf +++ b/src/Compiler/xlf/FSComp.txt.fr.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.it.xlf b/src/Compiler/xlf/FSComp.txt.it.xlf index 2b1e8e62c7a..1661484a63c 100644 --- a/src/Compiler/xlf/FSComp.txt.it.xlf +++ b/src/Compiler/xlf/FSComp.txt.it.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.ja.xlf b/src/Compiler/xlf/FSComp.txt.ja.xlf index 1a9e62e9a73..a5a04b7c1b0 100644 --- a/src/Compiler/xlf/FSComp.txt.ja.xlf +++ b/src/Compiler/xlf/FSComp.txt.ja.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.ko.xlf b/src/Compiler/xlf/FSComp.txt.ko.xlf index 0c67bd0dded..d419bd0d99b 100644 --- a/src/Compiler/xlf/FSComp.txt.ko.xlf +++ b/src/Compiler/xlf/FSComp.txt.ko.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.pl.xlf b/src/Compiler/xlf/FSComp.txt.pl.xlf index 61b93351bdd..65d582f3796 100644 --- a/src/Compiler/xlf/FSComp.txt.pl.xlf +++ b/src/Compiler/xlf/FSComp.txt.pl.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.pt-BR.xlf b/src/Compiler/xlf/FSComp.txt.pt-BR.xlf index 1b31443e280..93af3d22e75 100644 --- a/src/Compiler/xlf/FSComp.txt.pt-BR.xlf +++ b/src/Compiler/xlf/FSComp.txt.pt-BR.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.ru.xlf b/src/Compiler/xlf/FSComp.txt.ru.xlf index a21158f8391..b13f061163b 100644 --- a/src/Compiler/xlf/FSComp.txt.ru.xlf +++ b/src/Compiler/xlf/FSComp.txt.ru.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.tr.xlf b/src/Compiler/xlf/FSComp.txt.tr.xlf index ef179892d69..ad43bfe169b 100644 --- a/src/Compiler/xlf/FSComp.txt.tr.xlf +++ b/src/Compiler/xlf/FSComp.txt.tr.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf b/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf index bfbb2bd4593..d7013ad94e7 100644 --- a/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf +++ b/src/Compiler/xlf/FSComp.txt.zh-Hans.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation diff --git a/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf b/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf index 21cbc41d453..902612c1b64 100644 --- a/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf +++ b/src/Compiler/xlf/FSComp.txt.zh-Hant.xlf @@ -373,8 +373,8 @@ - Optimizes interpolated strings in certains cases, by lowering to concatenation - Optimizes interpolated strings in certains cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation + Optimizes interpolated strings in certain cases, by lowering to concatenation From 3bcb7452de2fc58b2d1d38f1021ede0879325bda Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Tue, 20 Feb 2024 16:08:39 +0100 Subject: [PATCH 16/20] Add a test, undo change in CheckFormatString --- src/Compiler/Checking/CheckFormatStrings.fs | 15 ++++---- .../Language/InterpolatedStringsTests.fs | 35 +++++++++++++------ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index 1d8523deb93..ec8b4ae2b91 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -384,12 +384,15 @@ let parseFormatStringInternal failwith (FSComp.SR.forFormatInvalidForInterpolated3()) let collectSpecifierLocation fragLine fragCol numStdArgs = - let numArgsForSpecifier = - numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) - specifierLocations.Add( - (Range.mkFileIndexRange m.FileIndex - (Position.mkPos fragLine startFragCol) - (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) + match context with + | Some _ -> + let numArgsForSpecifier = + numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine startFragCol) + (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) + | None -> () let ch = fmt[i] match ch with diff --git a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs index 068deecc9e9..7c82ac3d88e 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs @@ -132,10 +132,10 @@ type Foo () = [] // Test different number of interpolated string parts - [] - [] - [] - let ``Interpolated expressions are strings`` (strToPrint: string, expectedOut: string) = + [] + [] + [] + let ``Interpolated expressions are strings`` (strToPrint: string) = Fsx $""" let x = {strToPrint} printfn "%%s" x @@ -143,13 +143,28 @@ printfn "%%s" x |> withLangVersionPreview |> compileExeAndRun |> shouldSucceed - |> withStdOutContains expectedOut + |> withStdOutContains "abcde" + + let ``Multiline interpolated expression is a string`` () = + let strToPrint = String.Join(Environment.NewLine, "$\"\"\"a", "b", "c", "{\"d\"}", "e\"\"\"") + Fsx $""" +let x = {strToPrint} +printfn "%%s" x + """ + |> withLangVersionPreview + |> compileExeAndRun + |> shouldSucceed + |> withStdOutContains """a +b +c +d +e""" [] - [] - [] - [] - let ``In FormattableString, interpolated expressions are strings`` (formattableStr: string, expectedOut: string, argCount: int) = + [] + [] + [] + let ``In FormattableString, interpolated expressions are strings`` (formattableStr: string, argCount: int) = Fsx $""" let x = {formattableStr} : System.FormattableString assert(x.ArgumentCount = {argCount}) @@ -158,4 +173,4 @@ printfn "%%s" (System.Globalization.CultureInfo "en-US" |> x.ToString) |> withLangVersionPreview |> compileExeAndRun |> shouldSucceed - |> withStdOutContains expectedOut \ No newline at end of file + |> withStdOutContains "abcde" \ No newline at end of file From cbb7277fb89704179cc297e47785a54b46139e45 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 21 Feb 2024 18:46:23 +0100 Subject: [PATCH 17/20] Refactor based on review suggestions --- src/Compiler/Checking/CheckExpressions.fs | 157 ++++++++++------------ src/Compiler/Utilities/ReadOnlySpan.fs | 15 +++ src/Compiler/Utilities/ReadOnlySpan.fsi | 3 + 3 files changed, 89 insertions(+), 86 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index fc3855085ea..9db9634bdae 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -6,6 +6,7 @@ module internal FSharp.Compiler.CheckExpressions open System open System.Collections.Generic +open System.Text.RegularExpressions open Internal.Utilities.Collections open Internal.Utilities.Library @@ -145,27 +146,35 @@ exception InvalidInternalsVisibleToAssemblyName of badName: string * fileName: s // Used to decide if interpolated string can be lowered to a concat call. // We don't care about single- vs multi-$ strings here, because lexer took care of that already. //---------------------------------------------------------------------------------------------- -module private ParseString = - open System.Text.RegularExpressions - - // Regex for strings ending with unescaped '%s' - // For efficiency sake using lookbehind, starting from end of the string - let stringSpecifier = Regex(@"$(?<=(^|[^%])(%%)*%s)", RegexOptions.Compiled) - let EndsWithStringSpecifierNoFlags = stringSpecifier.IsMatch - - // Regex pattern for something like: %[flags][width][.precision][type] - // but match only if % is not escaped by another % - let formatSpecifier = - let sb = Text.StringBuilder(64) - sb.Append(@"(^|[^%])") |> ignore // Start with beginning of string or any char other than '%' - sb.Append(@"(%%)*%") |> ignore // followed by an odd number of '%' chars - sb.Append(@"[+-0 ]{0,3}") |> ignore // optionally followed by flags - sb.Append(@"(\d+)?") |> ignore // optionally followed by width - sb.Append(@"(\.\d+)?") |> ignore // optionally followed by .precision - sb.Append(@"[bscdiuxXoBeEfFgGMOAat]") |> ignore // and then a char that determines specifier's type - let pattern = sb.ToString() - Regex(pattern, RegexOptions.Compiled) - let HasFormatSpecifier = formatSpecifier.IsMatch +[] +let (|HasFormatSpecifier|_|) (s: string) = + if + Regex.IsMatch( + s, + """ + (^|[^%]) # Start with beginning of string or any char other than '%' + (%%)*% # followed by an odd number of '%' chars + [+-0 ]{0,3} # optionally followed by flags + (\d+)? # optionally followed by width + (\.\d+)? # optionally followed by .precision + [bscdiuxXoBeEfFgGMOAat] # and then a char that determines specifier's type + """, + RegexOptions.Compiled ||| RegexOptions.IgnorePatternWhitespace) + then + ValueSome HasFormatSpecifier + else + ValueNone + +let (|WithTrailingStringSpecifierRemoved|) (s: string) = + if s.EndsWith "%s" then + let i = s.AsSpan(0, s.Length - 2).LastIndexOfAnyExcept '%' + let diff = s.Length - 2 - i + if diff &&& 1 <> 0 then + s[..i] + else + s + else + s /// Compute the available access rights from a particular location in code let ComputeAccessRights eAccessPath eInternalsVisibleCompPaths eFamilyType = @@ -7363,72 +7372,48 @@ and TcInterpolatedStringExpr cenv (overallTy: OverallTy) env m tpenv (parts: Syn // Type check the expressions filling the holes let fillExprs, tpenv = TcExprsNoFlexes cenv env m tpenv argTys synFillExprs - // If all fill expressions are strings and there is less then 5 parts of the interpolated string total - // then we can use System.String.Concat instead of a sprintf call. - // First filter out all string parts that are empty or only have a '%s' string specifier, - // also get rid of the '%s' specifier right before the interpolation expression. - let nonEmptyParts = - parts - |> List.map (fun x -> - match x with - | SynInterpolatedStringPart.String(s, m) when ParseString.EndsWithStringSpecifierNoFlags s -> - SynInterpolatedStringPart.String(s.Substring(0, s.Length-2), shiftEnd 0 (-2) m) - | _ -> x) - |> List.filter (fun x -> - match x with - | SynInterpolatedStringPart.String(s, _) -> not <| System.String.IsNullOrEmpty s - | SynInterpolatedStringPart.FillExpr(_, _) -> true) - - // Then check if there are any format specifiers that might require actual formatting (as opposed to type checking) - let noSpecifiers stringParts = - stringParts - |> List.forall (fun x -> - match x with - | SynInterpolatedStringPart.FillExpr(expr, qualifiers) -> - match expr, qualifiers with - // Something like "...{x:hh}..." - | _, Some _ -> false - // Detect "x" part of "...{x,3}..." - | SynExpr.Tuple (false, [_; SynExpr.Const (SynConst.Int32 _align, _)], _, _), _ -> false - | _ -> true - | SynInterpolatedStringPart.String(s, _) -> not <| ParseString.HasFormatSpecifier s) - - if - (g.langVersion.SupportsFeature LanguageFeature.LowerInterpolatedStringToConcat) + // Take all interpolated string parts and typed fill expressions + // and convert them to typed expressions that can be used as args to System.String.Concat + // return an empty list if there are some format specifiers that make lowering to not applicable + let rec concatenable acc fillExprs parts = + match fillExprs, parts with + | [], [] -> + List.rev acc + | [], SynInterpolatedStringPart.FillExpr _ :: _ + | _, [] -> + // This should never happen, there will always be as many typed fill expressions + // as there are FillExprs in the interpolated string parts + error(InternalError("Mismatch in interpolation expression count", m)) + | _, SynInterpolatedStringPart.String (WithTrailingStringSpecifierRemoved "", _) :: parts -> + // If the string is empty (after trimming %s of the end), we skip it + concatenable acc fillExprs parts + + | _, SynInterpolatedStringPart.String (WithTrailingStringSpecifierRemoved HasFormatSpecifier, _) :: _ + | _, SynInterpolatedStringPart.FillExpr (_, Some _) :: _ + | _, SynInterpolatedStringPart.FillExpr (SynExpr.Tuple (isStruct = false; exprs = [_; SynExpr.Const (SynConst.Int32 _, _)]), _) :: _ -> + // There was a format specifier like %20s{..} or {..,20} or {x:hh}, which means we cannot simply concat + [] + + | _, SynInterpolatedStringPart.String (s & WithTrailingStringSpecifierRemoved trimmed, m) :: parts -> + let finalStr = trimmed.Replace("%%", "%") + concatenable (mkString g (shiftEnd 0 (finalStr.Length - s.Length) m) finalStr :: acc) fillExprs parts + + | fillExpr :: fillExprs, SynInterpolatedStringPart.FillExpr _ :: parts -> + concatenable (fillExpr :: acc) fillExprs parts + + let canLower = + g.langVersion.SupportsFeature LanguageFeature.LowerInterpolatedStringToConcat && isString - && (nonEmptyParts.Length < 5) - && (argTys |> List.forall (isStringTy g)) - && (noSpecifiers nonEmptyParts) - then - - let args = - // We already have the type-checked fill expressions from before - // so let's just weave them with the string parts in the right order - nonEmptyParts - |> List.fold (fun (fillExprs, acc) stringPart -> - match stringPart with - | SynInterpolatedStringPart.String(s, m) -> - let sb = System.Text.StringBuilder(s).Replace("%%", "%") - (fillExprs, (mkString g m (sb.ToString()))::acc) - | SynInterpolatedStringPart.FillExpr(_, _) -> - match fillExprs with - | expr::exprs -> (exprs, expr::acc) - | _ -> error(InternalError("Mismatch in interpolation expression count", m))) (fillExprs, []) - |> snd - |> List.rev - - assert (args.Length = nonEmptyParts.Length) - if args.Length = 4 then - (mkStaticCall_String_Concat4 g m args[0] args[1] args[2] args[3], tpenv) - elif args.Length = 3 then - (mkStaticCall_String_Concat3 g m args[0] args[1] args[2], tpenv) - elif args.Length = 2 then - (mkStaticCall_String_Concat2 g m args[0] args[1], tpenv) - elif args.Length = 1 then - args[0], tpenv - else - error(InternalError("Mismatch in arg count when lowering to Concat", m)) - else + && argTys |> List.forall (isStringTy g) + + let concatenableExprs = if canLower then concatenable [] fillExprs parts else [] + + match concatenableExprs with + | [p1; p2; p3; p4] -> mkStaticCall_String_Concat4 g m p1 p2 p3 p4, tpenv + | [p1; p2; p3] -> mkStaticCall_String_Concat3 g m p1 p2 p3, tpenv + | [p1; p2] -> mkStaticCall_String_Concat2 g m p1 p2, tpenv + | [p1] -> p1, tpenv + | _ -> let fillExprsBoxed = (argTys, fillExprs) ||> List.map2 (mkCallBox g m) diff --git a/src/Compiler/Utilities/ReadOnlySpan.fs b/src/Compiler/Utilities/ReadOnlySpan.fs index 05683eadb9e..0cfa8e3eb49 100644 --- a/src/Compiler/Utilities/ReadOnlySpan.fs +++ b/src/Compiler/Utilities/ReadOnlySpan.fs @@ -59,4 +59,19 @@ type ReadOnlySpanExtensions = i <- i - 1 if found then i else -1 + + [] + static member LastIndexOfAnyExcept(span: ReadOnlySpan, value: char) = + let mutable i = span.Length - 1 + let mutable found = false + + while not found && i >= 0 do + let c = span[i] + + if c <> value then + found <- true + else + i <- i - 1 + + if found then i else -1 #endif diff --git a/src/Compiler/Utilities/ReadOnlySpan.fsi b/src/Compiler/Utilities/ReadOnlySpan.fsi index 67591a03f88..b772ab58642 100644 --- a/src/Compiler/Utilities/ReadOnlySpan.fsi +++ b/src/Compiler/Utilities/ReadOnlySpan.fsi @@ -17,4 +17,7 @@ type internal ReadOnlySpanExtensions = [] static member LastIndexOfAnyInRange: span: ReadOnlySpan * lowInclusive: char * highInclusive: char -> int + + [] + static member LastIndexOfAnyExcept: span: ReadOnlySpan * value: char -> int #endif From b4f0bf393cf7f6a0adf9bd6ca9564cdc45cd2f0f Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 21 Feb 2024 18:46:41 +0100 Subject: [PATCH 18/20] Add more IL tests --- .../EmittedIL/StringFormatAndInterpolation.fs | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs index 1df9c914076..3038460f577 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StringFormatAndInterpolation.fs @@ -34,13 +34,31 @@ IL_0017: ret"""] #endif + [] + let ``Interpolated string with 2 parts consisting only of strings is lowered to concat`` () = + FSharp $""" +module StringFormatAndInterpolation + +let f (s: string) = $"ab{{s}}" + """ + |> withLangVersionPreview + |> compile + |> shouldSucceed + |> verifyIL [""" +IL_0000: ldstr "ab" +IL_0005: ldarg.0 +IL_0006: call string [runtime]System.String::Concat(string, + string) +IL_000b: ret"""] + [] let ``Interpolated string with 3 parts consisting only of strings is lowered to concat`` () = - let str = "$\"\"\"ab{\"c\"}d\"\"\"" + //let str = "$\"\"\"ab{\"c\"}d\"\"\"" FSharp $""" module StringFormatAndInterpolation -let str () = {str} +let c = "c" +let str = $"ab{{c}}d" """ |> withLangVersionPreview |> compile @@ -50,6 +68,27 @@ IL_0000: ldstr "ab" IL_0005: ldstr "c" IL_000a: ldstr "d" IL_000f: call string [runtime]System.String::Concat(string, + string, + string)"""] + + [] + let ``Interpolated string with 4 parts consisting only of strings is lowered to concat`` () = + let str = "$\"\"\"a{\"b\"}{\"c\"}d\"\"\"" + FSharp $""" +module StringFormatAndInterpolation + +let str () = {str} + """ + |> withLangVersionPreview + |> compile + |> shouldSucceed + |> verifyIL [""" +IL_0000: ldstr "a" +IL_0005: ldstr "b" +IL_000a: ldstr "c" +IL_000f: ldstr "d" +IL_0014: call string [runtime]System.String::Concat(string, + string, string, string) -IL_0014: ret"""] \ No newline at end of file +IL_0019: ret"""] \ No newline at end of file From eb62292ed6443cd830caaeb7db6d370268c5f443 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 21 Feb 2024 18:52:53 +0100 Subject: [PATCH 19/20] Add comments lost in refactoring --- src/Compiler/Checking/CheckExpressions.fs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 9db9634bdae..89268e1ee6e 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -151,6 +151,7 @@ let (|HasFormatSpecifier|_|) (s: string) = if Regex.IsMatch( s, + // Regex pattern for something like: %[flags][width][.precision][type] """ (^|[^%]) # Start with beginning of string or any char other than '%' (%%)*% # followed by an odd number of '%' chars @@ -165,6 +166,7 @@ let (|HasFormatSpecifier|_|) (s: string) = else ValueNone +// Removes trailing "%s" unless it was escaped by another '%' (checks for odd sequence of '%' before final "%s") let (|WithTrailingStringSpecifierRemoved|) (s: string) = if s.EndsWith "%s" then let i = s.AsSpan(0, s.Length - 2).LastIndexOfAnyExcept '%' From ea6c6ec18f2c6cbe0110daba246d7871e44820a0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 22 Feb 2024 11:26:35 +0000 Subject: [PATCH 20/20] Automated command ran: fantomas Co-authored-by: psfinaki <5451366+psfinaki@users.noreply.github.com> --- src/Compiler/Utilities/ReadOnlySpan.fs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Compiler/Utilities/ReadOnlySpan.fs b/src/Compiler/Utilities/ReadOnlySpan.fs index 0cfa8e3eb49..a01439bee63 100644 --- a/src/Compiler/Utilities/ReadOnlySpan.fs +++ b/src/Compiler/Utilities/ReadOnlySpan.fs @@ -68,10 +68,7 @@ type ReadOnlySpanExtensions = while not found && i >= 0 do let c = span[i] - if c <> value then - found <- true - else - i <- i - 1 + if c <> value then found <- true else i <- i - 1 if found then i else -1 #endif