From ec38a852a0bfc8e85b6d86baa1fa62f9de537e68 Mon Sep 17 00:00:00 2001 From: Mangel Maxime Date: Mon, 6 Jul 2026 15:28:42 +0200 Subject: [PATCH 1/2] perf(python): read union case fields directly instead of rebuilding Array --- pyrightconfig.json | 2 +- .../Python/Fable2Python.Transforms.fs | 69 +++++++++++-------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/pyrightconfig.json b/pyrightconfig.json index 6c0ea2cb63..df24032e47 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -9,9 +9,9 @@ "reportConstantRedefinition": false, // Fable preserves casing for variables starting with uppercase "reportUnnecessaryIsInstance": false, // Not a problem in generated code (look into this later) "reportUnnecessaryComparison": false, // Generated tests do this on purpose + "reportUnnecessaryCast": false, // Generated code for union case field need addional cast otherwise Pyrigh cannot infer the resolved typed "reportImportCycles": true, "reportMissingImports": true, - "reportUnnecessaryCast": true, "reportDuplicateImport": true, "reportOverlappingOverload": true, "reportInconsistentConstructor": true, diff --git a/src/Fable.Transforms/Python/Fable2Python.Transforms.fs b/src/Fable.Transforms/Python/Fable2Python.Transforms.fs index d5d76ec003..f5d4bce812 100644 --- a/src/Fable.Transforms/Python/Fable2Python.Transforms.fs +++ b/src/Fable.Transforms/Python/Fable2Python.Transforms.fs @@ -143,6 +143,33 @@ let getUnionCaseClassName $"%s{unionName}_%s{caseName}" +/// Resolves the expression referring to a union case's class (constructor / type), +/// handling both library types (Result, Choice - simple names from fable_library) +/// and user-defined unions (full case class name, imported from another module if needed). +let getUnionCaseRef (com: IPythonCompiler) ctx (entRef: Fable.EntityRef) (ent: Fable.Entity) (uci: Fable.UnionCase) = + if isLibraryUnionType entRef.FullName then + let caseName = getUnionCaseName uci + // Result uses "result" module, Choice uses "choice" module + let moduleName = + if entRef.FullName = Types.result then + "result" + else + "choice" + + libValue com ctx moduleName caseName + else + // User-defined union - use full case class name (UnionName_CaseName) + let caseClassName = getUnionCaseClassName com ent uci None + + match entRef.SourcePath with + | Some path when path <> com.CurrentFile -> + // Import from another module + let importPath = Path.getRelativeFileOrDirPath false com.CurrentFile false path + com.GetImportExpr(ctx, importPath, caseClassName) + | _ -> + // Local - just get identifier + com.GetIdentifierAsExpr(ctx, caseClassName) + let getUnionExprTag (com: IPythonCompiler) ctx r (fableExpr: Fable.Expr) = Expression.withStmts { let! expr = com.TransformAsExpr(ctx, fableExpr) @@ -532,32 +559,7 @@ let transformValue (com: IPythonCompiler) (ctx: Context) r value : Expression * // Get the union case let uci = ent.UnionCases |> List.item tag - - // Determine the import path based on the entity type - let caseRef = - // Library types (Result, Choice) use simple case names from fable_library - if isLibraryUnionType entRef.FullName then - let caseName = getUnionCaseName uci - // Result uses "result" module, Choice uses "choice" module - let moduleName = - if entRef.FullName = Types.result then - "result" - else - "choice" - - libValue com ctx moduleName caseName - else - // User-defined union - use full case class name (UnionName_CaseName) - let caseClassName = getUnionCaseClassName com ent uci None - // Check if it's from another file - match entRef.SourcePath with - | Some path when path <> com.CurrentFile -> - // Import from another module - let importPath = Path.getRelativeFileOrDirPath false com.CurrentFile false path - com.GetImportExpr(ctx, importPath, caseClassName) - | _ -> - // Local - just get identifier - com.GetIdentifierAsExpr(ctx, caseClassName) + let caseRef = getUnionCaseRef com ctx entRef ent uci // fsc represents a union case with no fields as a single shared instance, so e.g. // `LanguagePrimitives.PhysicalEquality X.A X.A` is true. Mirror that by reusing the @@ -1457,8 +1459,19 @@ let transformGet (com: IPythonCompiler) ctx range typ (fableExpr: Fable.Expr) ki | Fable.UnionField i -> Expression.withStmts { let! baseExpr = com.TransformAsExpr(ctx, fableExpr) - let! fieldsExpr = getExpr com ctx range baseExpr (Expression.stringConstant "fields") - let! finalExpr = getExpr com ctx range fieldsExpr (Expression.intConstant i.FieldIndex) + // Read the field directly by name (`x.width_`) instead of going through the + // `.fields` property (`x.fields[i]`), which rebuilds an Array on every access. + let ent = com.GetEntity(i.Entity) + let uci = ent.UnionCases |> List.item i.CaseIndex + let field = uci.UnionCaseFields |> List.item i.FieldIndex + let fieldName = field.Name |> Naming.toFieldSnakeCase |> Helpers.clean + // `fableExpr` is statically typed as the union base, which doesn't declare this + // field, so tell Pyright which case class we're reading from (cast is a no-op + // at runtime). See reportUnnecessaryCast in pyrightconfig.json. + let caseRef = getUnionCaseRef com ctx i.Entity ent uci + let cast = com.GetImportExpr(ctx, "typing", "cast") + let castedExpr = Expression.call (cast, [ caseRef; baseExpr ]) + let! finalExpr = getExpr com ctx range castedExpr (Expression.stringConstant fieldName) return finalExpr } From a840d4eff22ed8128e236bc61c5883855db137d1 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 8 Jul 2026 10:34:06 +0200 Subject: [PATCH 2/2] perf(python): emit union field reads via `narrow` helper instead of `typing.cast` (#4731) Co-authored-by: Claude Opus 4.8 (1M context) --- pyrightconfig.json | 2 +- .../Python/Fable2Python.Transforms.fs | 11 +++++----- src/fable-library-py/fable_library/union.py | 20 ++++++++++++++++++- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pyrightconfig.json b/pyrightconfig.json index df24032e47..6c0ea2cb63 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -9,9 +9,9 @@ "reportConstantRedefinition": false, // Fable preserves casing for variables starting with uppercase "reportUnnecessaryIsInstance": false, // Not a problem in generated code (look into this later) "reportUnnecessaryComparison": false, // Generated tests do this on purpose - "reportUnnecessaryCast": false, // Generated code for union case field need addional cast otherwise Pyrigh cannot infer the resolved typed "reportImportCycles": true, "reportMissingImports": true, + "reportUnnecessaryCast": true, "reportDuplicateImport": true, "reportOverlappingOverload": true, "reportInconsistentConstructor": true, diff --git a/src/Fable.Transforms/Python/Fable2Python.Transforms.fs b/src/Fable.Transforms/Python/Fable2Python.Transforms.fs index f5d4bce812..8363698ca3 100644 --- a/src/Fable.Transforms/Python/Fable2Python.Transforms.fs +++ b/src/Fable.Transforms/Python/Fable2Python.Transforms.fs @@ -1466,12 +1466,13 @@ let transformGet (com: IPythonCompiler) ctx range typ (fableExpr: Fable.Expr) ki let field = uci.UnionCaseFields |> List.item i.FieldIndex let fieldName = field.Name |> Naming.toFieldSnakeCase |> Helpers.clean // `fableExpr` is statically typed as the union base, which doesn't declare this - // field, so tell Pyright which case class we're reading from (cast is a no-op - // at runtime). See reportUnnecessaryCast in pyrightconfig.json. + // field, so tell Pyright which case class we're reading from via the library's + // `narrow` helper (a no-op at runtime). Keeping the "cast" inside `narrow` + // leaves generated code cast-free and preserves reportUnnecessaryCast. let caseRef = getUnionCaseRef com ctx i.Entity ent uci - let cast = com.GetImportExpr(ctx, "typing", "cast") - let castedExpr = Expression.call (cast, [ caseRef; baseExpr ]) - let! finalExpr = getExpr com ctx range castedExpr (Expression.stringConstant fieldName) + let narrow = libValue com ctx "union" "narrow" + let narrowedExpr = Expression.call (narrow, [ caseRef; baseExpr ]) + let! finalExpr = getExpr com ctx range narrowedExpr (Expression.stringConstant fieldName) return finalExpr } diff --git a/src/fable-library-py/fable_library/union.py b/src/fable-library-py/fable_library/union.py index 192f264582..db30dc1517 100644 --- a/src/fable-library-py/fable_library/union.py +++ b/src/fable-library-py/fable_library/union.py @@ -101,6 +101,24 @@ def CompareTo(self, other: Any) -> int: return -1 if self.tag < other.tag else 1 +def narrow[T](case: type[T], value: object) -> T: + """Narrow a union value to one of its case classes for the type checker. + + F# discriminated unions compile to a base ``Union`` class plus one case + class per case (e.g. ``Tree`` -> ``Tree_Leaf | Tree_Node``). Reading a + case-specific field such as ``x.left_`` requires the value to be typed as + that case class, but after a ``match`` on ``tag`` it is still statically + typed as the union. ``narrow(Tree_Node, x)`` re-types ``x`` so the field + read resolves, playing the same role as ``typing.cast`` but scoped to + unions. Keeping it in the library (instead of emitting ``cast`` inline) + lets generated code stay ``cast``-free and preserves ``reportUnnecessaryCast``. + + ``case`` is only used by the type checker to bind ``T``; this is an identity + function at runtime. + """ + return value # pyright: ignore[reportReturnType] + + @dataclass_transform() def tagged_union(tag: int): """Decorator for union case classes. @@ -135,4 +153,4 @@ def fields(self) -> Array[Any]: return decorator -__all__ = ["Union", "tagged_union"] +__all__ = ["Union", "narrow", "tagged_union"]