Implement interpolated strings via String.Concat - #19971
Open
charlesroddie wants to merge 17 commits into
Open
Conversation
A string-typed interpolated string is lowered to System.String.Concat of its parts rather than the reflection-based printf engine: a string-typed hole is passed through directly, any other plain hole is converted with `string x`, an aligned/formatted hole with `String.Format(InvariantCulture, ...)`, and a printf-specifier hole with `sprintf`. This removes the reflection dependency on the common path, so these interpolations become trim- and NativeAOT-compatible. This generalizes and replaces the language-version-gated String.Concat optimization (dotnet#16556), which only handled all-string holes: the lowering now applies to every string-typed interpolation, ungated. The reflection path is used only for PrintfFormat/FormattableString-typed interpolation. The syntax tree now carries each hole's formatting explicitly, so a printf specifier no longer leaks into an adjacent literal and alignment is no longer a fake tuple: type SynInterpolatedStringPart = | String of value: string * range: range | FillExpr of fillExpr: SynExpr * formatting: SynInterpolationFormatting type SynInterpolationFormatting = | DotNet of alignment: SynExpr option * format: Ident option | Printf of specifier: string * range: range Behavioural change: plain `{x}` holes now render with invariant culture (the F# `string` operator) rather than the current thread culture, matching `string`. Adds a NativeAOT regression test under tests/AheadOfTime/NativeAOT. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
✅ No release notes required |
This comment has been minimized.
This comment has been minimized.
charlesroddie
force-pushed
the
InterpolatedStringCollector
branch
from
June 18, 2026 21:51
e2debcd to
f571df9
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kerams
reviewed
Jun 19, 2026
Rewrite TcInterpolatedStringViaConcat to type-check each interpolation
part in place and convert it to a string expression, then String.Concat
them. This removes the parallel 'holeIsString' bool list and the
flat-fillExprs/dense-parts interleave entirely: 'build' now walks a
single list (the parts), threading only tpenv.
- Plain '{x}' holes are built directly in the typed tree: a string is
passed through raw (matching dotnet#16556's lean IL), anything else is
converted via the 'string' operator, emitted through a new
string_operator_info intrinsic + mkCallStringOperator helper.
- Aligned/formatted and printf holes are checked from a small synthesized
String.Format/sprintf expression, so name resolution still does the BCL
work.
- The function-value warning is re-homed per-hole.
Known follow-ups: ill-typed formatted holes currently report their error
twice (the formatted arm type-checks the hole once for the warning and
again inside String.Format); the warning wants to move to its own pass
over hole types, which also removes that double check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A bare '%s' on a string hole now lowers to a String.Concat passthrough
(reflection-free, AOT-clean), like a plain '{string}' hole, instead of
routing through sprintf. This fixes the regression where '$"%s{name}"'
went through the printf engine. Other specifiers (and '%5s' etc.) still
format via sprintf, and the '%s' string constraint is still enforced.
convertHole now returns a (string expression, may-be-null) pair: only a
raw string passthrough can be null, so a lone such arg coalesces via the
'string' operator (e.g. $"%s{null}" and $"{(s: string)}" -> ""), while
multi-arg cases rely on String.Concat mapping null to "".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The formatted/aligned hole path lowered via a synthesized String.Format that re-type-checked the hole expression a second time, which could duplicate an error in the hole and leak a spurious 'Format' overload-resolution error. Bind the already-checked, boxed hole value to a temporary and reference that from the synthesized String.Format instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
charlesroddie
commented
Jun 20, 2026
These printf specifiers act only as a type annotation - their value renders the
same through the 'string' operator as through the specifier - so constrain the
hole to the specifier's type and render it with 'string', as for a plain '{x}'
hole, instead of routing through the reflection-based 'sprintf'. This makes them
NativeAOT compatible. '%u' is excluded as it reinterprets a signed value as
unsigned and so does not match 'string'.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
T-Gro
reviewed
Jun 22, 2026
T-Gro
reviewed
Jun 22, 2026
| [NewObject (PrintfFormat`5, Value ("abc %P() def"), | ||
| NewArray (Object, Call (None, Box, [Value (1)])), | ||
| Value (<null>))])""" | ||
| """Call (None, Concat, |
Member
There was a problem hiding this comment.
This might be a breaking change for compilers of quoted representation.
CC @pezipink .
Member
There was a problem hiding this comment.
Yes, I am a little worried this leads to an issue similar to #19873
Contributor
Author
There was a problem hiding this comment.
That issue has two parts:
- A request for breaking changes in quotation representation to be communicated. I suppose that would go in an extra line in release notes here. I could add that here if it's the right place?
- A request for quotation representation to be similar structurally to the F# code written. In that respect the representation here is an improvement. The code does not intend to create a
PrintfFormat`5, does not intend to create an array, doesn't intend to box the value1, and doesn't intend to create anull, so the original is a poor representation. It does intend to create the pieces"abc ",1converted to a string, and" def"and join them together, and in that sense the new representation is good. It's not perfect because it doesn't capture the fact that the user is using an interpolated string expression, but it is a big improvement.
T-Gro
reviewed
Jun 22, 2026
T-Gro
reviewed
Jun 22, 2026
T-Gro
reviewed
Jun 22, 2026
The NativeAOT check only asserted that publish and execution succeeded, so a change that silently altered interpolated-string rendering would pass unnoticed. The test program now checks each rendering against an expected string literal, printing a "FAILED" line on mismatch and "Finished" last; check.ps1 asserts the output is exactly "Finished". This follows the existing Trimming/check.ps1 pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
charlesroddie
force-pushed
the
InterpolatedStringCollector
branch
from
June 22, 2026 16:39
4cfdd83 to
7d2a246
Compare
Cover the lowerings the old (<=4 string parts) optimization never reached: - more than 4 parts -> System.String.Concat(string[]) array overload - string holes (incl. a string-returning method) concatenated directly, with no spurious string conversion or null check - a single float hole -> invariant-culture ToString (the rendering behaviour change), with no concat - a single bool hole -> ToString via the non-IFormattable path, with no concat Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
charlesroddie
added a commit
to charlesroddie/fsharp
that referenced
this pull request
Jun 22, 2026
Addresses review feedback: keep the column-aligned layout of the surrounding intrinsic table. Also makes these two lines byte-identical to the same intrinsic added by dotnet#19971, so a future merge resolves cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
charlesroddie
commented
Jun 23, 2026
T-Gro
reviewed
Jul 24, 2026
Contributor
|
🔍 Tooling Safety Check — Affects-Compiler-Output, Affects-Test-Tooling
|
abonie
pushed a commit
that referenced
this pull request
Jul 30, 2026
* Add a compiler intrinsic for the 'string' operator Adds string_operator_info / mkCallStringOperator so generated code can call Operators.string. These lines are duplicated by the interpolated-string PR (#19971); kept identical there so a future merge resolves cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Generate a match-based ToString for unions under --reflectionfree Under --reflectionfree the union ToString previously emitted nothing, so DUs fell back to Object.ToString() (the namespace-qualified type name). Instead generate a match over the cases that builds "CaseName(f0, f1, ...)" using the 'string' operator on each field, via a TypedTree expression fed to CodeGenMethodForExpr. This recurses naturally into nested unions and is reflection-free. The default (sprintf "%+A") path is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Extract mkStringConcat helper for arity-dispatched String.Concat The "concatenate a list of string exprs, picking the cheapest String.Concat overload by arity" pattern was duplicated in CheckExpressions (interpolation lowering) and the optimizer, and our new union ToString used the array overload unconditionally. Extract mkStringConcat into TypedTreeOps.ExprOps and route all three through it. This also lets single-field union cases emit Concat3 instead of allocating a string[] (IlxGen runs after the optimizer, so nothing else would collapse that array form). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix generated union ToString for generic unions The match-based ToString body is a TypedTree expression codegen'd via CodeGenMethodForExpr, but it was built with `eenv`, which lacks the tycon's type parameters. For generic unions this produced wrong IL: the wrong case branch (always the null-as-true-value case) or a NullReferenceException for single-case unions. Use `eenvinner` (the per-tycon environment) so the generic method body resolves its type parameters. The old sprintf path was unaffected because it emits raw IL off the pre-built ilThisTy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Render union ToString fields like option (null -> "null") To make a generated union ToString consistent with how option/list format their contents (LanguagePrimitives.anyToStringShowingNull), format each field as: if (box field) is non-null then 'string field' else "null". Previously a null field rendered as "" (the 'string' operator's null behaviour). Generated inline rather than calling anyToStringShowingNull, which is internal to FSharp.Core and so not callable from user-compiled code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Tidy reflection-free union ToString tests Normalize union declarations to a leading '|', use System.Console.WriteLine instead of printfn (the printf machinery is what these changes move away from), and make the null-field test compare the union's rendering directly against option's rather than asserting a fixed string. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add reflection-free ToString to Result and Choice Result and Choice had no ToString override, so they fell back to the compiler-generated sprintf "%+A" one, which uses reflection. Give them hand-written overrides mirroring option/list (String.Concat + anyToStringShowingNull), e.g. Ok 5 -> "Ok(5)", Choice1Of2 7 -> "Choice1Of2(7)". This is reflection-free / AOT-friendly and consistent with option's "Some(x)" rendering. Note: this changes the observable ToString of Result/Choice from the "%A"-style "Ok 5" to "Ok(5)". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Generate a single-line ToString for records under --reflectionfree Records previously fell back to Object.ToString() (the namespace-qualified type name) under --reflectionfree. Generate "{ F1 = v1; F2 = v2 }" on a single line (no line breaks, unlike sprintf "%+A"), with fields formatted like union fields (null -> "null", otherwise via 'string'). Factor the shared field formatter and ToString-method emission out of the union path. The default (sprintf "%+A") path is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Update FSharp.Core surface-area baselines for Result/Choice ToString Result and Choice`2..7 now declare an explicit ToString() override, so they appear in the public surface area. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add release notes Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Generate a single-line ToString for anonymous records under --reflectionfree Drive anonymous-record ToString through the synthetic record tycon (already built for equality/comparison) rather than sprintf "%A", so under --reflectionfree it renders "{| Name = value; ... |}" on a single line. GenRecordToStringMethod now takes open/close brace strings ("{ "/" }" for records, "{| "/" |}" for anonymous records). The default (non-reflection-free) codegen path is unchanged and still falls back to sprintf "%+A". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Test that a hand-written ToString override is kept under --reflectionfree Addresses review feedback: generation is gated on `not (HasMember "ToString")`, so a user-defined ToString on a union or record wins over the generated one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rename ToString generators for clarity Addresses review feedback: distinguish the reflective sprintf path from the structural one. GenPrintingMethod -> GenSprintfPrintingMethod (the sprintf "%+A" ToString/get_Message), GenToStringMethodFromExpr -> EmitToStringMethodDef. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Restore tabular layout for string_operator_info in TcGlobals Addresses review feedback: keep the column-aligned layout of the surrounding intrinsic table. Also makes these two lines byte-identical to the same intrinsic added by #19971, so a future merge resolves cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add reflection-free ToString tests for field shapes, structs, anon records and recursion Covers DU field shapes (multiple fields vs a single tuple field), explicit vs unnamed field names rendering identically, struct unions/records, anonymous and struct anonymous records, and finite recursive/nesting types. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add EmittedIL tests for reflection-free record and union ToString Locks in the IL emitted under --reflectionfree: each field is boxed and rendered through Operators.ToString with a null guard, and the parts are joined with String.Concat (array form for the record, 3-arg form for the single-field union case). Nullary union cases return the bare case name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Generate reflection-free ToString in the augmentation phase The structural ToString for --reflectionfree records and unions was built in IlxGen, after the optimizer, so its per-field 'string' operator calls were never inlined: each value-type field was boxed and rendered through the generic Operators.ToString, behind a null guard that is dead for a value type. Move the generation into the type-augmentation phase (alongside Equals/GetHashCode/CompareTo) so the body flows through the optimizer. The 'string' operator is now specialised - a value-type field renders via a direct, allocation-free invariant-culture ToString with no boxing and no null guard (reference fields keep the guard so null still renders as "null"). The shared body builders live in AugmentTypeDefinitions; anonymous record types are synthesized too late for augmentation, so they keep generating in IlxGen but reuse the same builder. Output is unchanged; the EmittedIL baselines are updated to the leaner IL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Guard generated reflection-free ToString against deep-recursion overflow The augmentation-generated structural ToString recurses into fields, so a deeply nested value can exhaust the stack with an uncatchable StackOverflowException. Emit RuntimeHelpers.EnsureSufficientExecutionStack() at method entry (as C# records do in PrintMembers) so it throws a catchable InsufficientExecutionStackException instead, when the runtime provides the method. The guard is skipped for types whose every field is a flat primitive (integer/float/decimal/string/char/bool/unit/enum), which cannot recurse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Test the reflection-free ToString deep-recursion guard A 1,000,000-deep value's generated ToString throws a catchable InsufficientExecutionStackException rather than hard-crashing the process. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * revert ToString additions to fsharp.core types * Remove stale FSharp.Core release note for the reverted Result/Choice ToString Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix code formatting in IlxGen.fs (dotnet fantomas) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * don't use quoted name * int version of reflectionfree-printing doc * doc tweaks * Link release note to the printing doc and cover anonymous records Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test backticks * Share the ToString recursion guard with anonymous records The guard lived in MakeBindingsForToStringAugmentation, which anonymous records bypass: they are synthesized too late for type augmentation and reach mkRecdToString from IlxGen instead. Deep nesting overflowed the stack rather than raising InsufficientExecutionStackException. Move it into mkToStringRecursionGuard, applied inside mkRecdToString and mkUnionToString, so every caller of the body builders gets it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add EmittedIL baselines for struct and anonymous record ToString Struct records and unions read fields off the this pointer and switch on the tag, and the anonymous record path is generated separately in IlxGen, so each gets its own baseline. The anonymous baseline omits the field reads: they name the anonymous type, whose mangled name is not stable across compilations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix empty anonymous record ToString rendering a doubled space The open/close braces carry inner spaces ("{| " and " |}"); with no fields they abut and render "{| |}". Trim the leading space when the field list is empty, matching %A's "{| |}". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * tidy comment --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ollector Resolves the duplicated 'string' operator intrinsic: dotnet#19976 added an identical v_string_operator_info / mkCallStringOperator, so the two copies collapse into one.
dotnet#19976 extracted the arity-dispatched String.Concat builder into TypedTreeOps.ExprOps, so the interpolation lowering no longer needs its own copy. The only case it keeps is the lone possibly-null string, which has no Concat to map null to "". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The old guard matched a ':fmt' qualifier only; an alignment hole was protected by accident, because ',3' was encoded as a tuple in the fill expression and the tuple rule kept the parens. Lifting the alignment into SynInterpolationFormatting removed that tuple, so match the alignment here too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hole was checked against 'string', so under --checknulls a nullable string gave FS3261 where 'sprintf "%s"' does not. Share the specifier's type as CheckFormatStrings.stringFormatTy and check the hole against that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both apps statically link FSharp.Compiler.Service, so the added lowering code shifts them by one 512-byte alignment unit. CI reported both values. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
abonie
approved these changes
Jul 31, 2026
abonie
enabled auto-merge (squash)
July 31, 2026 10:12
abonie
disabled auto-merge
July 31, 2026 10:13
abonie
self-requested a review
July 31, 2026 10:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements fsharp/fslang-suggestions#1108 (comment), so that interpolated strings, where possible, are implemented via
String.Concat. The benefits are:DefaultInterpolatedStringHandlerfsharp/fslang-suggestions#1108 (comment), with perf measurements)There are some problems with the current implementation, and this PR takes a moderate approach, resolving some of them but retaining others to keep a broadly backwards-compatible implementation.
stringorPrintfFormat<...>. The latter is an unfortunate addition designed to allow expressions likeprintf $"...", working around the lack ofprintfunctions (fsharp/fslang-suggestions#1092). This should be marked obsolete whenprintfunctions are added.a) The parser handled these only partially: it left the specifier (e.g.
%f) inside the preceding string component, leaving the compiler step to locate and process it. In this PR the specifier is split off and attached to its hole in a parser helper instead. So it's improved but still messy.b) Any hole with these expressions goes through the previous reflection-based route.
stringare culture-independent. This is adjusted to match existing behaviour, using thestringfunction. This is in keeping with similar changes that have moved towardsstringbehaviour. (See the related discussion ofstringvsToStringbehaviour in fsharp/fslang-suggestions#919.)A NativeAOT test
A test under
tests/AheadOfTime/NativeAOT(wired into the Windows trimming CI job) AOT-publishes a program that uses interpolation. Plain and .NET-format holes ({x},{x:F2},{x,6}) are AOT compatible. Printf-format holes (%d{x}) still route throughsprintf, so they remain reflection-based and fail AOT with IL2026/IL2070/IL3050 — see the commented-out examples in the test.This would be a good place to add other currently AOT-incompatible expressions for similar future fixes.
Notes on the LowerInterpolatedStringToConcat feature flag
This work extends and supersedes the work under the LowerInterpolatedStringToConcat flag, an "optimization that lowers string interpolation into a call to concat iff there are at most 4 string parts and all fill expressions are strings".
The reason why this feature was gated is unclear since it's just an optimization, but it's unclear what to do with this gate here. Options:
stringin culture-independence) is a fix to existing behaviour.