Skip to content

Fix 9565 - The nullable interop feature caused an issue with byrefs when enabled. #9566

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.

Sign up for GitHub

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

Closed
KevinRansom wants to merge 3 commits into from
Closed

Fix 9565 - The nullable interop feature caused an issue with byrefs when enabled. #9566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e1bc3f
Fix 9565
KevinRansom Jun 25, 2020
cfb667c
Fix 9564
KevinRansom Jun 26, 2020
efc8986
typo
KevinRansom Jun 26, 2020
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
116 changes: 61 additions & 55 deletions src/fsharp/MethodCalls.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,82 +1150,88 @@ let GetDefaultExpressionForOptionalArg tcFieldInit g (calledArg: CalledArg) eCal
let callerArg = CallerArg(calledArgTy, mMethExpr, false, expr)
preBinder, { NamedArgIdOpt = None; CalledArg = calledArg; CallerArg = callerArg }


Copy link
Contributor

@TIHan TIHan Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't need an extra space?

let MakeNullableExprIfNeeded (infoReader: InfoReader) calledArgTy callerArgTy callerArgExpr m =
let g = infoReader.g
let amap = infoReader.amap
if isNullableTy g callerArgTy then
callerArgExpr
None
else
// this happens when the caller and callee are not both nullable, I.e the scenario enabled by LanguageFeature.NullableOptionalInterop
tryLanguageFeatureError g.langVersion LanguageFeature.NullableOptionalInterop m
let calledNonOptTy = destNullableTy g calledArgTy
let minfo = GetIntrinsicConstructorInfosOfType infoReader m calledArgTy |> List.head
let callerArgExprCoerced = mkCoerceIfNeeded g calledNonOptTy callerArgTy callerArgExpr
MakeMethInfoCall amap m minfo [] [callerArgExprCoerced]
Some (MakeMethInfoCall amap m minfo [] [callerArgExprCoerced])

// Adjust all the optional arguments, filling in values for defaults,
let AdjustCallerArgForOptional tcFieldInit eCallerMemberName (infoReader: InfoReader) (assignedArg: AssignedCalledArg<_>) =

let g = infoReader.g
let callerArg = assignedArg.CallerArg
let (CallerArg(callerArgTy, m, isOptCallerArg, callerArgExpr)) = callerArg
let calledArg = assignedArg.CalledArg
let calledArgTy = calledArg.CalledArgumentType
match calledArg.OptArgInfo with
| NotOptional when not (g.langVersion.SupportsFeature LanguageFeature.NullableOptionalInterop) ->
if isOptCallerArg then errorR(Error(FSComp.SR.tcFormalArgumentIsNotOptional(), m))
assignedArg

| _ ->
// callerArgExprOpt
// For caller/callee's where we can generate nullable compensation using MakeNullableExprIfNeeded
// generate assignedarg.
// callerArgExprOpt either:
// Some the generated compensation expression, or None
let callerArgExprOpt =
match calledArg.OptArgInfo with
| NotOptional ->
// T --> Nullable<T> widening at callsites
if isOptCallerArg then errorR(Error(FSComp.SR.tcFormalArgumentIsNotOptional(), m))
if isNullableTy g calledArgTy then
MakeNullableExprIfNeeded infoReader calledArgTy callerArgTy callerArgExpr m
else
None

let callerArgExpr2 =
match calledArg.OptArgInfo with
| NotOptional ->
// T --> Nullable<T> widening at callsites
if isOptCallerArg then errorR(Error(FSComp.SR.tcFormalArgumentIsNotOptional(), m))
if isNullableTy g calledArgTy then
MakeNullableExprIfNeeded infoReader calledArgTy callerArgTy callerArgExpr m
| CallerSide dfltVal ->
let calledArgTy = calledArg.CalledArgumentType

if isOptCallerArg then
// CSharpMethod(?x=b)
if isOptionTy g callerArgTy then
if isNullableTy g calledArgTy then
// CSharpMethod(?x=b) when 'b' has optional type and 'x' has nullable type --> CSharpMethod(x=Option.toNullable b)
Some (mkOptionToNullable g m (destOptionTy g callerArgTy) callerArgExpr)
else
// CSharpMethod(?x=b) when 'b' has optional type and 'x' has non-nullable type --> CSharpMethod(x=Option.defaultValue DEFAULT v)
let _wrapper, defaultExpr = GetDefaultExpressionForCallerSideOptionalArg tcFieldInit g calledArg calledArgTy dfltVal eCallerMemberName m
let ty = destOptionTy g callerArgTy
Some (mkOptionDefaultValue g m ty defaultExpr callerArgExpr)
else
callerArgExpr

| CallerSide dfltVal ->
let calledArgTy = calledArg.CalledArgumentType

if isOptCallerArg then
// CSharpMethod(?x=b)
if isOptionTy g callerArgTy then
if isNullableTy g calledArgTy then
// CSharpMethod(?x=b) when 'b' has optional type and 'x' has nullable type --> CSharpMethod(x=Option.toNullable b)
mkOptionToNullable g m (destOptionTy g callerArgTy) callerArgExpr
else
// CSharpMethod(?x=b) when 'b' has optional type and 'x' has non-nullable type --> CSharpMethod(x=Option.defaultValue DEFAULT v)
let _wrapper, defaultExpr = GetDefaultExpressionForCallerSideOptionalArg tcFieldInit g calledArg calledArgTy dfltVal eCallerMemberName m
let ty = destOptionTy g callerArgTy
mkOptionDefaultValue g m ty defaultExpr callerArgExpr
else
// This should be unreachable but the error will be reported elsewhere
callerArgExpr
// This should be unreachable but the error will be reported elsewhere
None
else
if isNullableTy g calledArgTy then
// CSharpMethod(x=b) when 'x' has nullable type
// CSharpMethod(x=b) when both 'x' and 'b' have nullable type --> CSharpMethod(x=b)
// CSharpMethod(x=b) when 'x' has nullable type and 'b' does not --> CSharpMethod(x=Nullable(b))
MakeNullableExprIfNeeded infoReader calledArgTy callerArgTy callerArgExpr m
else
// CSharpMethod(x=b) --> CSharpMethod(?x=b)
None

| CalleeSide ->
if isOptCallerArg then
// CSharpMethod(?x=b) --> CSharpMethod(?x=b)
None
else
// CSharpMethod(x=b) when CSharpMethod(A) --> CSharpMethod(?x=Some(b :> A))
if isOptionTy g calledArgTy then
let calledNonOptTy = destOptionTy g calledArgTy
let callerArgExprCoerced = mkCoerceIfNeeded g calledNonOptTy callerArgTy callerArgExpr
Some (mkSome g calledNonOptTy callerArgExprCoerced m)
else
if isNullableTy g calledArgTy then
// CSharpMethod(x=b) when 'x' has nullable type
// CSharpMethod(x=b) when both 'x' and 'b' have nullable type --> CSharpMethod(x=b)
// CSharpMethod(x=b) when 'x' has nullable type and 'b' does not --> CSharpMethod(x=Nullable(b))
MakeNullableExprIfNeeded infoReader calledArgTy callerArgTy callerArgExpr m
else
// CSharpMethod(x=b) --> CSharpMethod(?x=b)
callerArgExpr

| CalleeSide ->
if isOptCallerArg then
// CSharpMethod(?x=b) --> CSharpMethod(?x=b)
callerArgExpr
else
// CSharpMethod(x=b) when CSharpMethod(A) --> CSharpMethod(?x=Some(b :> A))
if isOptionTy g calledArgTy then
let calledNonOptTy = destOptionTy g calledArgTy
let callerArgExprCoerced = mkCoerceIfNeeded g calledNonOptTy callerArgTy callerArgExpr
mkSome g calledNonOptTy callerArgExprCoerced m
else
assert false
callerArgExpr // defensive code - this case is unreachable
assert false
None // defensive code - this case is unreachable

match callerArgExprOpt with
| None -> assignedArg
| Some callerArgExpr2 ->
let callerArg2 = CallerArg(tyOfExpr g callerArgExpr2, m, isOptCallerArg, callerArgExpr2)
{ assignedArg with CallerArg=callerArg2 }

Expand Down
18 changes: 14 additions & 4 deletions tests/fsharp/tests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,20 @@ module CoreTests =
begin
use testOkFile = fileguard cfg "test.ok"

fsc cfg "%s -o:test.exe -g" cfg.fsc_flags ["test.fsx"]
fsc cfg "%s -o:test.exe -g --langversion:4.7" cfg.fsc_flags ["test.fsx"]

singleNegTest cfg "test"
singleVersionedNegTest cfg "4.7" "test"
exec cfg ("." ++ "test.exe") ""

testOkFile.CheckExists()
end

begin
use testOkFile = fileguard cfg "test.ok"

fsc cfg "%s -o:test.exe -g --langversion:5.0" cfg.fsc_flags ["test.fsx"]

singleVersionedNegTest cfg "5.0" "test"

exec cfg ("." ++ "test.exe") ""

Expand Down Expand Up @@ -482,7 +493,6 @@ module CoreTests =
let ``enum-FSI_BASIC`` () = singleTestBuildAndRun' "core/enum" FSI_BASIC

#if !NETCOREAPP

// Requires winforms will not run on coreclr
[<Test>]
let controlWpf () = singleTestBuildAndRun' "core/controlwpf" FSC_BASIC
Expand Down Expand Up @@ -3009,4 +3019,4 @@ module OverloadResolution =
let [<Test>] ``neg_known_return_type_and_known_type_arguments`` () = singleNegTest (testConfig' "typecheck/overloads") "neg_known_return_type_and_known_type_arguments"
let [<Test>] ``neg_generic_known_argument_types`` () = singleNegTest (testConfig' "typecheck/overloads") "neg_generic_known_argument_types"
let [<Test>] ``neg_tupled_arguments`` () = singleNegTest (testConfig' "typecheck/overloads") "neg_tupled_arguments"
#endif
#endif