Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3584 #3587

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
29 changes: 10 additions & 19 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,8 +865,11 @@ 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 ->
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 @@ -1516,13 +1512,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 +1737,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 +2393,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 +2874,7 @@ module Util =
let args =
info.CurriedParameterGroups
|> List.concat
// |> discardUnitArg
// |> FSharp2Fable.Util.discardUnitArg
|> List.toArray

let argsLen = Array.length args
Expand Down
28 changes: 10 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,10 @@ 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 -> [], []

// TODO: discardUnitArg may still be needed in some cases
// | 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 +3224,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
32 changes: 14 additions & 18 deletions src/Fable.Transforms/Rust/Fable2Rust.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,24 +1158,19 @@ 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

/// 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 +2146,7 @@ module Util =
{ ctx with RequiresSendSync = isSendSync
IsParamByRefPreferred = isByRefPreferred }

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 +2617,10 @@ 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 = 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 @@ -385,7 +385,7 @@

let mutable mutableValue = 0

let rec recursive1 = delay (fun () -> recursive2())

Check warning on line 388 in tests/Js/Main/MiscTests.fs

View workflow job for this annotation

GitHub Actions / build-javascript

This and other recursive references to the object(s) being defined will be checked for initialization-soundness at runtime through the use of a delayed reference. This is because you are defining one or more recursive objects, rather than recursive functions. This warning may be suppressed by using '#nowarn "40"' or '--nowarn:40'.
and recursive2 =
mutableValue <- 5
fun () -> mutableValue <- mutableValue * 2
Expand Down Expand Up @@ -477,9 +477,17 @@

let sideEffect() = ()

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

let tests =
testList "Miscellaneous" [

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

#if FABLE_COMPILER
#if !FABLE_COMPILER_JAVASCRIPT
testCase "Fable.JsonProvider works" <| fun _ ->
Expand Down Expand Up @@ -1069,7 +1077,7 @@
| 2 -> x <- "2"
| 3 | 4 -> x <- "3" // Multiple cases are allowed
// | 5 | 6 as j -> x <- string j // This prevents the optimization
| 4 -> x <- "4" // Unreachable cases are removed

Check warning on line 1080 in tests/Js/Main/MiscTests.fs

View workflow job for this annotation

GitHub Actions / build-javascript

This rule will never be matched
| _ -> x <- "?"
equal "3" x

Expand Down
Loading
Loading