Skip to content

Commit

Permalink
Bugfix :: Fix optimizer bug where field.Index included compiler gener…
Browse files Browse the repository at this point in the history
…ated static fields (#18280)

* Fix optimizer bug where field.Index included compiler generated static fields

* notes added
  • Loading branch information
T-Gro authored Feb 3, 2025
1 parent 58560f8 commit 5b910af
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.300.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### Fixed
* Fix Realsig+ generates nested closures with incorrect Generic ([Issue #17797](https://github.com/dotnet/fsharp/issues/17797), [PR #17877](https://github.com/dotnet/fsharp/pull/17877))
* Fix optimizer internal error for records with static fields ([Issue #18165](https://github.com/dotnet/fsharp/issues/18165), [PR #18280](https://github.com/dotnet/fsharp/pull/18280))
* Fix internal error when missing measure attribute in an unsolved measure typar. ([Issue #7491](https://github.com/dotnet/fsharp/issues/7491), [PR #18234](https://github.com/dotnet/fsharp/pull/18234)==
* Set `Cancellable.token` from async computation ([Issue #18235](https://github.com/dotnet/fsharp/issues/18235), [PR #18238](https://github.com/dotnet/fsharp/pull/18238))
* Cancellable: only cancel on OCE with own token ([PR #18277](https://github.com/dotnet/fsharp/pull/18277))
Expand Down
20 changes: 15 additions & 5 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4273,6 +4273,19 @@ type UnionCaseRef =

override x.ToString() = x.CaseName

let findLogicalFieldIndexOfRecordField (tcref:TyconRef) (id:string) =
let arr = tcref.AllFieldsArray

// We are skipping compiler generated fields such as "init@5" from index calculation
let rec go originalIdx skippedItems =
if originalIdx >= arr.Length then error(InternalError(sprintf "field %s not found in type %s" id tcref.LogicalName, tcref.Range))
else
let currentItem = arr[originalIdx]
if currentItem.LogicalName = id then (originalIdx-skippedItems)
else go (originalIdx + 1) (skippedItems + (if currentItem.IsCompilerGenerated && currentItem.IsStatic then 1 else 0))

go 0 0

/// Represents a reference to a field in a record, class or struct
[<NoEquality; NoComparison; StructuredFormatDisplay("{DebugText}")>]
type RecdFieldRef =
Expand Down Expand Up @@ -4316,11 +4329,8 @@ type RecdFieldRef =

member x.Index =
let (RecdFieldRef(tcref, id)) = x
try
// REVIEW: this could be faster, e.g. by storing the index in the NameMap
tcref.AllFieldsArray |> Array.findIndex (fun rfspec -> rfspec.LogicalName = id)
with :? KeyNotFoundException ->
error(InternalError(sprintf "field %s not found in type %s" id tcref.LogicalName, tcref.Range))
findLogicalFieldIndexOfRecordField tcref id


[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
member x.DebugText = x.ToString()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module Test

// https://github.com/dotnet/fsharp/issues/18165

type FooBar =
{ xyz : string }
static let staticLet = 1

let doThing (foo : FooBar) =
let bar = { foo with xyz = foo.xyz }
let baz = { bar with xyz = bar.xyz }
printf "%O" baz

doThing { xyz = "" }
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ init R 2
1
2"""

[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"RecordOptimizerRegression.fs"|])>]
let ``Static let - record optimizer regression`` compilation =
compilation
|> withOptimize
|> verifyCompileAndRun
|> shouldSucceed
|> withStdOutContains """{ xyz = "" }"""



[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"LowercaseDuTest.fs"|])>]
let ``Static let - lowercase DU`` compilation =
compilation
Expand Down

0 comments on commit 5b910af

Please sign in to comment.