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

code cleanup prior to optional interop improvements #7276

Merged
merged 3 commits into from
Jul 27, 2019
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: 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