Skip to content

Commit

Permalink
Fixes #3584
Browse files Browse the repository at this point in the history
  • Loading branch information
ncave committed Nov 14, 2023
1 parent 37feeeb commit 0179c0d
Show file tree
Hide file tree
Showing 13 changed files with 160 additions and 127 deletions.
2 changes: 2 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
"src/fcs-fable"
],
"rust-analyzer.linkedProjects": [
"build/fable-library-rust/Cargo.toml",
"build/tests/Rust/Cargo.toml",
"temp/fable-library-rust/Cargo.toml",
"temp/tests/Rust/Cargo.toml"
],
Expand Down
4 changes: 4 additions & 0 deletions src/Fable.Cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

#### All

* Fix #3584: Unit type compiles to undeclared variable (by @ncave)

#### Rust

* Added `Guid.TryParse`, `Guid.ToByteArray` (by @ncave)
Expand Down
30 changes: 16 additions & 14 deletions src/Fable.Transforms/Dart/Fable2Dart.fs
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,6 @@ module Util =
let fn = com.GetImportIdent(ctx, memberName, modulePath, Fable.Any)
Expression.invocationExpression(fn.Expr, args, transformType com ctx t)

let discardUnitArg (args: Fable.Ident list) =
match args with
| [] -> []
| [unitArg] when unitArg.Type = Fable.Unit -> []
| [thisArg; unitArg] when thisArg.IsThisArgument && unitArg.Type = Fable.Unit -> [thisArg]
| args -> args

let addErrorAndReturnNull (com: Compiler) (range: SourceLocation option) (error: string) =
addError com [] range error
NullLiteral Dynamic |> Literal
Expand Down Expand Up @@ -244,8 +237,11 @@ module Util =
type NamedTailCallOpportunity(_com: IDartCompiler, ctx, name, args: Fable.Ident list) =
// Capture the current argument values to prevent delayed references from getting corrupted,
// for that we use block-scoped ES2015 variable declarations. See #681, #1859
let argIds = discardUnitArg args |> List.map (fun arg ->
getUniqueNameInDeclarationScope ctx (arg.Name + "_mut"))
let argIds =
args
|> FSharp2Fable.Util.discardUnitArg
|> List.map (fun arg ->
getUniqueNameInDeclarationScope ctx (arg.Name + "_mut"))
interface ITailCallOpportunity with
member _.Label = name
member _.Args = argIds
Expand Down Expand Up @@ -359,12 +355,16 @@ module Util =
statements1 @ statements2 @ [Statement.continueStatement(tc.Label)]

let transformCallArgs (com: IDartCompiler) ctx (info: ArgsInfo) =

let paramsInfo, thisArg, args =
match info with
| NoCallInfo args -> None, None, args
| NoCallInfo args ->
let args = FSharp2Fable.Util.dropUnitCallArg args []
None, None, args
| CallInfo callInfo ->
let args = FSharp2Fable.Util.dropUnitCallArg callInfo.Args callInfo.SignatureArgTypes
let paramsInfo = callInfo.MemberRef |> Option.bind com.TryGetMember |> Option.map getParamsInfo
paramsInfo, callInfo.ThisArg, callInfo.Args
paramsInfo, callInfo.ThisArg, args

let unnamedArgs, namedArgs =
paramsInfo
Expand All @@ -383,7 +383,6 @@ module Util =

let unnamedArgs =
match unnamedArgs, paramsInfo with
| [Fable.Value(Fable.UnitConstant,_)], _ -> []
| args, Some paramsInfo ->
let argsLen = args.Length
let parameters = paramsInfo.Parameters
Expand Down Expand Up @@ -833,7 +832,10 @@ module Util =
let invocation =
match args with
| [] -> Expression.invocationExpression(callee, t)
| args -> (callee, args) ||> List.fold (fun e arg -> Expression.invocationExpression(e, [arg], t))
| args ->
(callee, args)
||> List.fold (fun expr arg ->
Expression.invocationExpression(expr, [arg], t))
let statements2, capturedExpr = resolveExpr returnStrategy invocation
statements @ statements2, capturedExpr

Expand Down Expand Up @@ -1062,7 +1064,7 @@ module Util =
let tailcallChance = Option.map (fun name ->
NamedTailCallOpportunity(com, ctx, name, args) :> ITailCallOpportunity) name

let args = discardUnitArg args
let args = FSharp2Fable.Util.discardUnitArg args
let mutable isTailCallOptimized = false
let varsInScope = args |> List.map (fun a -> a.Name) |> HashSet
let ctx =
Expand Down
28 changes: 24 additions & 4 deletions src/Fable.Transforms/FSharp2Fable.Util.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,26 @@ module Util =
open TypeHelpers
open Identifiers

let isUnitArg (ident: Fable.Ident) =
ident.IsCompilerGenerated
&& ident.Type = Fable.Unit
// && (ident.DisplayName.StartsWith("unitVar") || ident.DisplayName.Contains("@"))

let discardUnitArg (args: Fable.Ident list) =
match args with
| [] -> []
| [arg] when isUnitArg arg -> []
| [thisArg; arg] when thisArg.IsThisArgument && isUnitArg arg -> [thisArg]
| args -> args

let dropUnitCallArg (args: Fable.Expr list) (argTypes: Fable.Type list) =
match args, argTypes with
// Don't remove unit arg if a generic is expected
| [MaybeCasted(Fable.Value(Fable.UnitConstant,_))], [Fable.GenericParam _] -> args
| [MaybeCasted(Fable.Value(Fable.UnitConstant,_))], _ -> []
| [Fable.IdentExpr ident], _ when isUnitArg ident -> []
| _ -> args

let makeFunctionArgs com ctx (args: FSharpMemberOrFunctionOrValue list) =
let ctx, args =
((ctx, []), args)
Expand Down Expand Up @@ -2168,10 +2188,10 @@ module Util =
let makeValueFrom (com: IFableCompiler) (ctx: Context) r (v: FSharpMemberOrFunctionOrValue) =
let typ = makeType ctx.GenericArgs v.FullType
match v, v.DeclaringEntity with
| _ when typ = Fable.Unit ->
if com.Options.Verbosity = Verbosity.Verbose && not v.IsCompilerGenerated then // See #1516
$"Value %s{v.DisplayName} is replaced with unit constant"
|> addWarning com ctx.InlinePath r
| _ when typ = Fable.Unit && v.IsCompilerGenerated ->
// if com.Options.Verbosity = Verbosity.Verbose && not v.IsCompilerGenerated then // See #1516
// $"Value %s{v.DisplayName} is replaced with unit constant"
// |> addWarning com ctx.InlinePath r
Fable.Value(Fable.UnitConstant, r)
| Emitted com r typ None emitted, _ -> emitted
| Imported com r typ None imported -> imported
Expand Down
24 changes: 6 additions & 18 deletions src/Fable.Transforms/Fable2Babel.fs
Original file line number Diff line number Diff line change
Expand Up @@ -849,13 +849,6 @@ module Util =
| Fable.LetRec(bindings, body) -> Some(bindings, body)
| _ -> None

let discardUnitArg (args: Fable.Ident list) =
match args with
| [] -> []
| [unitArg] when unitArg.Type = Fable.Unit -> []
| [thisArg; unitArg] when thisArg.IsThisArgument && unitArg.Type = Fable.Unit -> [thisArg]
| args -> args

let getUniqueNameInRootScope (ctx: Context) name =
let name = (name, Naming.NoMemberPart) ||> Naming.sanitizeIdent (fun name ->
ctx.UsedNames.RootScope.Contains(name)
Expand All @@ -872,7 +865,7 @@ module Util =
type NamedTailCallOpportunity(_com: Compiler, ctx, name, args: Fable.Ident list) =
// Capture the current argument values to prevent delayed references from getting corrupted,
// for that we use block-scoped ES2015 variable declarations. See #681, #1859
let argIds = args |> discardUnitArg |> List.map (fun arg ->
let argIds = args |> FSharp2Fable.Util.discardUnitArg |> List.map (fun arg ->
getUniqueNameInDeclarationScope ctx (arg.Name + "_mut"))
interface ITailCallOpportunity with
member _.Label = name
Expand Down Expand Up @@ -1516,13 +1509,8 @@ module Util =
Expression.newExpression(classExpr, [||])

let transformCallArgs (com: IBabelCompiler) ctx (callInfo: Fable.CallInfo) (memberInfo: Fable.MemberFunctionOrValue option) =
let args =
match callInfo.Args, callInfo.SignatureArgTypes with
// Don't remove unit arg if a generic is expected, TypeScript will complain
| [Fable.Value(Fable.UnitConstant,_)], [Fable.GenericParam _] -> callInfo.Args
| [Fable.Value(Fable.UnitConstant,_)], _ -> []
| _ -> callInfo.Args

let args = FSharp2Fable.Util.dropUnitCallArg callInfo.Args callInfo.SignatureArgTypes
let paramsInfo = Option.map getParamsInfo memberInfo

let args =
Expand Down Expand Up @@ -1746,14 +1734,14 @@ module Util =

let transformCurriedApply com ctx range (TransformExpr com ctx applied) args =
(applied, args)
||> List.fold (fun e arg ->
||> List.fold (fun expr arg ->
match arg with
// TODO: If arg type is unit but it's an expression with potential
// side-effects, we need to extract it and execute it before the call
| Fable.Value(Fable.UnitConstant,_) -> []
| Fable.IdentExpr ident when ident.Type = Fable.Unit -> []
| TransformExpr com ctx arg -> [arg]
|> callFunction com ctx range e [])
|> callFunction com ctx range expr [])

let transformCallAsStatements com ctx range t returnStrategy callee callInfo =
let argsLen (i: Fable.CallInfo) =
Expand Down Expand Up @@ -2402,7 +2390,7 @@ module Util =
let tailcallChance =
Option.map (fun name ->
NamedTailCallOpportunity(com, ctx, name, args) :> ITailCallOpportunity) name
let args = discardUnitArg args
let args = FSharp2Fable.Util.discardUnitArg args
let declaredVars = ResizeArray()
let mutable isTailCallOptimized = false
let ctx =
Expand Down Expand Up @@ -2883,7 +2871,7 @@ module Util =
let args =
info.CurriedParameterGroups
|> List.concat
// |> discardUnitArg
// |> FSharp2Fable.Util.discardUnitArg
|> List.toArray

let argsLen = Array.length args
Expand Down
26 changes: 8 additions & 18 deletions src/Fable.Transforms/Python/Fable2Python.fs
Original file line number Diff line number Diff line change
Expand Up @@ -997,17 +997,6 @@ module Util =
| Fable.Delegate (args, body, _, []) -> Some(args, body)
| _ -> None

let discardUnitArg (args: Fable.Ident list) =
match args with
| [] -> []
| [ unitArg ] when unitArg.Type = Fable.Unit -> []
| [ thisArg; unitArg ] when
thisArg.IsThisArgument
&& unitArg.Type = Fable.Unit
->
[ thisArg ]
| args -> args

let getUniqueNameInRootScope (ctx: Context) name =
let name =
(name, Naming.NoMemberPart)
Expand Down Expand Up @@ -1036,7 +1025,8 @@ module Util =
// for that we use block-scoped ES2015 variable declarations. See #681, #1859
// TODO: Local unique ident names
let argIds =
discardUnitArg args
args
|> FSharp2Fable.Util.discardUnitArg
|> List.map (fun arg ->
let name = getUniqueNameInDeclarationScope ctx (arg.Name + "_mut")
// Ignore type annotation here as it generates unnecessary typevars
Expand Down Expand Up @@ -1856,8 +1846,9 @@ module Util =
Expression.call (Expression.name name), [ stmt ] @ stmts

let transformCallArgs (com: IPythonCompiler) ctx (callInfo: Fable.CallInfo) : Expression list * Keyword list * Statement list =

let args = FSharp2Fable.Util.dropUnitCallArg callInfo.Args callInfo.SignatureArgTypes
let paramsInfo = callInfo.MemberRef |> Option.bind com.TryGetMember |> Option.map getParamsInfo
let args = callInfo.Args

let args, objArg, stmts =
paramsInfo
Expand Down Expand Up @@ -1890,8 +1881,7 @@ module Util =

let args, stmts' =
match args with
| []
| [ MaybeCasted (Fable.Value (Fable.UnitConstant, _)) ] -> [], []
| [] -> [], []
| args when hasSpread ->
match List.rev args with
| [] -> [], []
Expand Down Expand Up @@ -2065,8 +2055,8 @@ module Util =
match arg with
// TODO: If arg type is unit but it's an expression with potential
// side-effects, we need to extract it and execute it before the call
| Fable.Value(Fable.UnitConstant,_) -> [], []
| Fable.IdentExpr ident when ident.Type = Fable.Unit -> [], []
// | Fable.Value(Fable.UnitConstant,_) -> [], []
// | Fable.IdentExpr ident when ident.Type = Fable.Unit -> [], []
| TransformExpr com ctx (arg, stmts') -> [arg], stmts'
callFunction range applied args [], stmts @ stmts')

Expand Down Expand Up @@ -3232,7 +3222,7 @@ module Util =
let tailcallChance =
Option.map (fun name -> NamedTailCallOpportunity(com, ctx, name, args) :> ITailCallOpportunity) name

let args = discardUnitArg args
let args = FSharp2Fable.Util.discardUnitArg args

/// Removes `_mut` or `_mut_1` suffix from the identifier name
let cleanName (input: string) = Regex.Replace(input, @"_mut(_\d+)?$", "")
Expand Down
41 changes: 23 additions & 18 deletions src/Fable.Transforms/Rust/Fable2Rust.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,24 +1158,26 @@ module Util =
| _ -> None

let isUnitArg (ident: Fable.Ident) =
ident.IsCompilerGenerated &&
ident.Type = Fable.Unit &&
(ident.DisplayName.StartsWith("unitVar") || ident.DisplayName.Contains("@"))
ident.IsCompilerGenerated
&& ident.Type = Fable.Unit
&& (ident.DisplayName.StartsWith("unitVar") || ident.DisplayName.Contains("@"))

let discardUnitArg (genArgs: Fable.Type list) (args: Fable.Ident list) =
match genArgs, args with
| [Fable.Unit], [arg] -> args // don't drop unit arg when generic arg is unit
| _, [] -> []
| _, [arg] when isUnitArg arg -> []
| _, [thisArg; arg] when thisArg.IsThisArgument && isUnitArg arg -> [thisArg]
| _, args -> args

let dropUnitCallArg (genArgs: Fable.Type list) (args: Fable.Expr list) =
match genArgs, args with
| [Fable.Unit], [arg] -> args // don't drop unit arg when generic arg is unit
| _, [MaybeCasted(Fable.Value(Fable.UnitConstant, _))] -> []
| _, [Fable.IdentExpr ident] when isUnitArg ident -> []
| _, args -> args
| _ ->
match args with
| [] -> []
| [arg] when isUnitArg arg -> []
| [thisArg; arg] when thisArg.IsThisArgument && isUnitArg arg -> [thisArg]
| args -> args

// let dropUnitCallArg (genArgs: Fable.Type list) (args: Fable.Expr list) =
// match genArgs, args with
// | [Fable.Unit], [arg] -> args // don't drop unit arg when generic arg is unit
// | _, [MaybeCasted(Fable.Value(Fable.UnitConstant, _))] -> []
// | _, [Fable.IdentExpr ident] when isUnitArg ident -> []
// | _, args -> args

/// Fable doesn't currently sanitize attached members/fields so we do a simple sanitation here.
/// Should this be done in FSharp2Fable step?
Expand Down Expand Up @@ -2151,7 +2153,8 @@ module Util =
{ ctx with RequiresSendSync = isSendSync
IsParamByRefPreferred = isByRefPreferred }

let args = dropUnitCallArg callInfo.GenericArgs callInfo.Args
// let args = dropUnitCallArg callInfo.GenericArgs callInfo.Args
let args = FSharp2Fable.Util.dropUnitCallArg callInfo.Args callInfo.SignatureArgTypes
let args = transformCallArgs com ctx args callInfo.SignatureArgTypes argParams

match calleeExpr with
Expand Down Expand Up @@ -2622,9 +2625,11 @@ module Util =
optimizeTailCall com ctx r tc args
| _ ->
let callee = transformCallee com ctx calleeExpr
(callee, args) ||> List.fold (fun c arg ->
let args = dropUnitCallArg [] [arg]
callFunction com ctx r c args)
(callee, args)
||> List.fold (fun expr arg ->
// let args = dropUnitCallArg [] [arg]
let args = FSharp2Fable.Util.dropUnitCallArg [arg] []
callFunction com ctx r expr args)

let makeUnionCasePat unionCaseName fields =
if List.isEmpty fields then
Expand Down
2 changes: 1 addition & 1 deletion tests/Dart/src/ArrayTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ let tests () =
ys.[0] + ys.[1]
|> equal 3.

testCase "Array.concat works with strings" <| fun test ->
testCase "Array.concat works with strings" <| fun () ->
[| [| "One" |]; [| "Two" |] |]
|> Array.concat
|> List.ofArray
Expand Down
2 changes: 1 addition & 1 deletion tests/Js/Main/ArrayTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ let tests =
ys.[0] + ys.[1]
|> equal 3.

testCase "Array.concat works with strings" <| fun test ->
testCase "Array.concat works with strings" <| fun () ->
[| [| "One" |]; [| "Two" |] |]
|> Array.concat
|> List.ofArray
Expand Down
8 changes: 8 additions & 0 deletions tests/Js/Main/MiscTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -477,9 +477,17 @@ let inline inlineLambdaWithAnonRecord callback =

let sideEffect() = ()

let inline inlineToString (f: 'T -> string): 'T -> string =
let unused = f
fun a -> string a

let tests =
testList "Miscellaneous" [

// testCase "Generic unit args work" <| fun _ -> // #3584
// let to_str = inlineToString (fun (props: unit) -> "s")
// to_str () |> equal (string ())

#if FABLE_COMPILER
#if !FABLE_COMPILER_JAVASCRIPT
testCase "Fable.JsonProvider works" <| fun _ ->
Expand Down
Loading

0 comments on commit 0179c0d

Please sign in to comment.