Skip to content

Commit

Permalink
code cleanup prior to optional interop improvements (#7276)
Browse files Browse the repository at this point in the history
* add test cases we need to make work

* cleanup method call argument processing
  • Loading branch information
dsyme authored and KevinRansom committed Jul 27, 2019
1 parent 969a9c4 commit cc60ed6
Show file tree
Hide file tree
Showing 21 changed files with 664 additions and 518 deletions.
2 changes: 1 addition & 1 deletion src/fsharp/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,7 @@ and SolveTypeIsNonNullableValueType (csenv: ConstraintSolverEnv) ndeep m2 trace
| _ ->
let underlyingTy = stripTyEqnsAndMeasureEqns g ty
if isStructTy g underlyingTy then
if isAppTy g underlyingTy && tyconRefEq g g.system_Nullable_tcref (tcrefOfAppTy g underlyingTy) then
if isNullableTy g underlyingTy then
return! ErrorD (ConstraintSolverError(FSComp.SR.csTypeParameterCannotBeNullable(), m, m))
else
return! ErrorD (ConstraintSolverError(FSComp.SR.csGenericConstructRequiresStructType(NicePrint.minimalStringOfType denv ty), m, m2))
Expand Down
451 changes: 375 additions & 76 deletions src/fsharp/MethodCalls.fs

Large diffs are not rendered by default.

35 changes: 25 additions & 10 deletions src/fsharp/TastOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -839,16 +839,6 @@ let tryNiceEntityRefOfTyOption ty =
| TType_app (tcref, _) -> Some tcref
| TType_measure (Measure.Con tcref) -> Some tcref
| _ -> None

let (|NullableTy|_|) g ty =
match tryAppTy g ty with
| ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> Some tyarg
| _ -> None

let (|StripNullableTy|) g ty =
match tryAppTy g ty with
| ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> tyarg
| _ -> ty

let mkInstForAppTy g ty =
match tryAppTy g ty with
Expand Down Expand Up @@ -3125,6 +3115,31 @@ let destOptionTy g ty =
| ValueSome ty -> ty
| ValueNone -> failwith "destOptionTy: not an option type"

let isNullableTy (g: TcGlobals) ty =
match tryDestAppTy g ty with
| ValueNone -> false
| ValueSome tcref -> tyconRefEq g g.system_Nullable_tcref tcref

let tryDestNullableTy g ty =
match argsOfAppTy g ty with
| [ty1] when isNullableTy g ty -> ValueSome ty1
| _ -> ValueNone

let destNullableTy g ty =
match tryDestNullableTy g ty with
| ValueSome ty -> ty
| ValueNone -> failwith "destNullableTy: not a Nullable type"

let (|NullableTy|_|) g ty =
match tryAppTy g ty with
| ValueSome (tcref, [tyarg]) when tyconRefEq g tcref g.system_Nullable_tcref -> Some tyarg
| _ -> None

let (|StripNullableTy|) g ty =
match tryDestNullableTy g ty with
| ValueSome tyarg -> tyarg
| _ -> ty

let isLinqExpressionTy g ty =
match tryDestAppTy g ty with
| ValueNone -> false
Expand Down
11 changes: 10 additions & 1 deletion src/fsharp/TastOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,7 @@ val mkVoidPtrTy : TcGlobals -> TType
/// Build a single-dimensional array type
val mkArrayType : TcGlobals -> TType -> TType

/// Determine is a type is an option type
/// Determine if a type is an option type
val isOptionTy : TcGlobals -> TType -> bool

/// Take apart an option type
Expand All @@ -1397,6 +1397,15 @@ val destOptionTy : TcGlobals -> TType -> TType
/// Try to take apart an option type
val tryDestOptionTy : TcGlobals -> TType -> ValueOption<TType>

/// Determine is a type is a System.Nullable type
val isNullableTy : TcGlobals -> TType -> bool

/// Try to take apart a System.Nullable type
val tryDestNullableTy: TcGlobals -> TType -> ValueOption<TType>

/// Take apart a System.Nullable type
val destNullableTy: TcGlobals -> TType -> TType

/// Determine if a type is a System.Linq.Expression type
val isLinqExpressionTy : TcGlobals -> TType -> bool

Expand Down
553 changes: 153 additions & 400 deletions src/fsharp/TypeChecker.fs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/fsharp/xlf/FSComp.txt.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
</trans-unit>
<trans-unit id="followingPatternMatchClauseHasWrongType">
<source>All branches of a pattern match expression must return values of the same type as the first branch, which here is '{0}'. This branch returns a value of type '{1}'.</source>
<target state="translated">Todas las ramas de una expresión de coincidencia de patrón deben devolver valores del mismo tipo. La primera rama devolvió un valor de tipo "{0}", pero esta rama devolvió un valor de tipo "\{1 \}".</target>
<target state="new">All branches of a pattern match expression must return values of the same type as the first branch, which here is '{0}'. This branch returns a value of type '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="patternMatchGuardIsNotBool">
Expand Down
26 changes: 26 additions & 0 deletions tests/fsharp/core/fsfromfsviacs/lib3.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,29 @@ public static void SomeMethod() { }

}
}
namespace CSharpOptionalParameters
{
// This should be preferred over the same type in lib2.cs
public class SomeClass
{
public SomeClass() { }
public static int MethodTakingOptionals(int x = 3, string y = "abc", double d = 5.0)
{
return x + y.Length + (int) d;
}
public static int MethodTakingNullableOptionalsWithDefaults(int? x = 3, string y = "abc", double? d = 5.0)
{
return (x.HasValue ? x.Value : -100) + y.Length + (int) (d.HasValue ? d.Value : 0.0);
}
public static int MethodTakingNullableOptionals(int? x = null, string y = null, double? d = null)
{
int length;
if (y == null)
length = -1;
else
length = y.Length;
return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
}
}

}
63 changes: 47 additions & 16 deletions tests/fsharp/core/fsfromfsviacs/test.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,53 @@ let _ = test "structunion394b36" (Lib.NestedStructUnionsTests.testPattern3mut(u2

// F# option implicit converter tests

let testFsOpt() =
let testOpt (t : 'T option) =
test (sprintf "fsimplicitconv (%A)" t) (ApiWrapper.ConsumeOptionalParam<'T>(t) = t)

testOpt(Option<int>.None)
testOpt(Some 42)

// check that implicit conversion of optionals does
// differentiate between 'null' and 'Some null'
testOpt(Option<string>.None)
testOpt(Option<string>.Some null)
testOpt(Some "")
testOpt(Some "test")

testFsOpt()

module TestConsumeOptionalParameter =
let testFsOpt() =
let testOpt (t : 'T option) =
test (sprintf "fsimplicitconv (%A)" t) (ApiWrapper.ConsumeOptionalParam<'T>(t) = t)

testOpt(Option<int>.None)
testOpt(Some 42)

// check that implicit conversion of optionals does
// differentiate between 'null' and 'Some null'
testOpt(Option<string>.None)
testOpt(Option<string>.Some null)
testOpt(Some "")
testOpt(Some "test")

testFsOpt()

module TestConsumeCSharpOptionalParameter =
open System
open CSharpOptionalParameters
check "csoptional23982f31" (SomeClass.MethodTakingOptionals()) 11
check "csoptional23982f32" (SomeClass.MethodTakingOptionals(x = 6)) 14
check "csoptional23982f33" (SomeClass.MethodTakingOptionals(y = "aaaaaa")) 14
check "csoptional23982f34" (SomeClass.MethodTakingOptionals(d = 8.0)) 14

check "csoptional23982f41" (SomeClass.MethodTakingNullableOptionalsWithDefaults()) 11
check "csoptional23982f42" (SomeClass.MethodTakingNullableOptionalsWithDefaults(x = Nullable 6)) 14
check "csoptional23982f43" (SomeClass.MethodTakingNullableOptionalsWithDefaults(y = "aaaaaa")) 14
check "csoptional23982f44" (SomeClass.MethodTakingNullableOptionalsWithDefaults(d = Nullable 8.0)) 14

check "csoptional23982f51" (SomeClass.MethodTakingNullableOptionals()) -3
check "csoptional23982f52" (SomeClass.MethodTakingNullableOptionals(x = Nullable 6)) 4
check "csoptional23982f53" (SomeClass.MethodTakingNullableOptionals(y = "aaaaaa")) 4
check "csoptional23982f54" (SomeClass.MethodTakingNullableOptionals(d = Nullable 8.0)) 6

// These require https://github.com/fsharp/fslang-suggestions/issues/774 to be implemented
//check "csoptional23982f3no" (SomeClass.SomeMethod(?x = Some 6)) 14
//check "csoptional23982f3no" (SomeClass.SomeMethod(?y = Some "aaaaaa")) 14
//check "csoptional23982f3no" (SomeClass.SomeMethod(?d = Some 8.0)) 14
//check "csoptional23982f3no" (SomeClass.SomeMethod(?x = None)) 11
//check "csoptional23982f3no" (SomeClass.SomeMethod(?y = None)) 11
//check "csoptional23982f3no" (SomeClass.SomeMethod(?d = None)) 11

//check "csoptional23982f42" (SomeClass.MethodTakingNullableOptionalsWithDefaults(x = 6)) 14
//check "csoptional23982f44" (SomeClass.MethodTakingNullableOptionalsWithDefaults(d = 8.0)) 14
//check "csoptional23982f52" (SomeClass.MethodTakingNullableOptionals(x = 6)) 4
//check "csoptional23982f54" (SomeClass.MethodTakingNullableOptionals(d = 8.0)) 6

module NestedStructPatternMatchingAcrossAssemblyBoundaries =
open Lib.NestedStructUnionsTests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,8 +1828,9 @@ CONSIDER: get this from CodeDom</note>
</trans-unit>
<trans-unit id="RSE_GraphicSizeFormat">
<source>{0} x {1}</source>
<target state="translated">{0} x {1}</target>
<target state="needs-review-translation">{0} x {1}</target>
<note>Format string for showing a graphic's size

# {0} = width (as an integer)
# {1} = height (as an integer)
#Example, for a bitmap of width=123, height = 456, the English version of this string would be "123x456"</note>
Expand Down

0 comments on commit cc60ed6

Please sign in to comment.