Skip to content

Commit

Permalink
Lower interpolation into a call to concat (#16556)
Browse files Browse the repository at this point in the history
* Add unit tests for interp strings with string expr

Sanity check that lowering to concat does not break these simple cases

* Add IL test for lowering to concat

* 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)

* 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).

* Adjust expected length of codegen

* Filter out empty string parts

E.g. $"{x}{y}" has 5 string parts, including 3 empty strings

* Detect format specifiers better

There were false positives before

* Refactor, improve regex

* Unrelated indentation fix in parser

* Use language feature flag

* FSComp.txt auto-update

* Add release notes

* Add langversion flag to some unit tests

* Fix typo in src/Compiler/FSComp.txt

Co-authored-by: Petr <[email protected]>

* Update .xlf and undo change to CheckFormatStrings

* Add a test, undo change in CheckFormatString

* Refactor based on review suggestions

* Add more IL tests

* Add comments lost in refactoring

* Automated command ran: fantomas

  Co-authored-by: psfinaki <[email protected]>

---------

Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
4 people authored Feb 22, 2024
1 parent 57098f6 commit 2de1f68
Show file tree
Hide file tree
Showing 25 changed files with 295 additions and 21 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
4 changes: 4 additions & 0 deletions docs/release-notes/.Language/preview.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
113 changes: 97 additions & 16 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -140,6 +141,43 @@ exception StandardOperatorRedefinitionWarning of string * range

exception InvalidInternalsVisibleToAssemblyName of badName: string * fileName: string option

//----------------------------------------------------------------------------------------------
// 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.
//----------------------------------------------------------------------------------------------
[<return: Struct>]
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
[+-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

// 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 '%'
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 =
AccessibleFrom (eAccessPath :: eInternalsVisibleCompPaths, eFamilyType)
Expand Down Expand Up @@ -7336,25 +7374,68 @@ 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 fillExprsBoxed = (argTys, fillExprs) ||> List.map2 (mkCallBox g m)
// 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
[]

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)
| _, 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

let fmtExpr = MakeMethInfoCall cenv.amap m newFormatMethod [] [mkString g m printfFormatString; argsExpr; percentATysExpr] None
| fillExpr :: fillExprs, SynInterpolatedStringPart.FillExpr _ :: parts ->
concatenable (fillExpr :: acc) fillExprs parts

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
let canLower =
g.langVersion.SupportsFeature LanguageFeature.LowerInterpolatedStringToConcat
&& isString
&& 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)

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

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 ->
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 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."
Expand Down
3 changes: 3 additions & 0 deletions src/Compiler/Facilities/LanguageFeatures.fs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type LanguageFeature =
| WarningIndexedPropertiesGetSetSameType
| WarningWhenTailCallAttrOnNonRec
| BooleanReturningAndReturnTypeDirectedPartialActivePattern
| LowerInterpolatedStringToConcat

/// LanguageVersion management
type LanguageVersion(versionText) =
Expand Down Expand Up @@ -197,6 +198,7 @@ type LanguageVersion(versionText) =
LanguageFeature.WarningWhenTailCallAttrOnNonRec, previewVersion
LanguageFeature.UnionIsPropertiesVisible, previewVersion
LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern, previewVersion
LanguageFeature.LowerInterpolatedStringToConcat, previewVersion
]

static let defaultLanguageVersion = LanguageVersion("default")
Expand Down Expand Up @@ -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 =
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Facilities/LanguageFeatures.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type LanguageFeature =
| WarningIndexedPropertiesGetSetSameType
| WarningWhenTailCallAttrOnNonRec
| BooleanReturningAndReturnTypeDirectedPartialActivePattern
| LowerInterpolatedStringToConcat

/// LanguageVersion management
type LanguageVersion =
Expand Down
12 changes: 12 additions & 0 deletions src/Compiler/Utilities/ReadOnlySpan.fs
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,16 @@ type ReadOnlySpanExtensions =
i <- i - 1

if found then i else -1

[<Extension>]
static member LastIndexOfAnyExcept(span: ReadOnlySpan<char>, 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
3 changes: 3 additions & 0 deletions src/Compiler/Utilities/ReadOnlySpan.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ type internal ReadOnlySpanExtensions =

[<Extension>]
static member LastIndexOfAnyInRange: span: ReadOnlySpan<char> * lowInclusive: char * highInclusive: char -> int

[<Extension>]
static member LastIndexOfAnyExcept: span: ReadOnlySpan<char> * value: char -> int
#endif
4 changes: 2 additions & 2 deletions src/Compiler/pars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2de1f68

Please sign in to comment.