From 9c092b1003f01ab5999aaba75063ca179659353b Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 10 Dec 2019 03:21:35 -0800 Subject: [PATCH 1/6] Trying to fix span type abbrev --- src/fsharp/PostInferenceChecks.fs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fsharp/PostInferenceChecks.fs b/src/fsharp/PostInferenceChecks.fs index c718a94665f..475c848d39e 100644 --- a/src/fsharp/PostInferenceChecks.fs +++ b/src/fsharp/PostInferenceChecks.fs @@ -357,6 +357,10 @@ let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, v match visitAppTyOpt with | Some visitAppTy -> visitAppTy (tcref, tinst) | None -> () + + match tcref.TypeAbbrev with + | Some tya -> CheckTypeDeep cenv f g env isInner tya + | _ -> () | TType_anon (anonInfo, tys) -> RecordAnonRecdInfo cenv anonInfo CheckTypesDeep cenv f g env tys From 61994d3a436f04d02a48a4f8f850989ce37c37fb Mon Sep 17 00:00:00 2001 From: TIHan Date: Tue, 10 Dec 2019 18:55:40 -0800 Subject: [PATCH 2/6] Fixed type abbrev issue on span --- src/fsharp/PostInferenceChecks.fs | 6 +-- tests/fsharp/Compiler/CompilerAssert.fs | 2 +- tests/fsharp/Compiler/Language/SpanTests.fs | 52 +++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/fsharp/PostInferenceChecks.fs b/src/fsharp/PostInferenceChecks.fs index 475c848d39e..7b62a1093bc 100644 --- a/src/fsharp/PostInferenceChecks.fs +++ b/src/fsharp/PostInferenceChecks.fs @@ -332,7 +332,7 @@ let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, v | _ -> ()) | _ -> () - let ty = stripTyparEqns ty + let ty = stripTyEqns g ty visitTy ty match ty with @@ -357,10 +357,6 @@ let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, v match visitAppTyOpt with | Some visitAppTy -> visitAppTy (tcref, tinst) | None -> () - - match tcref.TypeAbbrev with - | Some tya -> CheckTypeDeep cenv f g env isInner tya - | _ -> () | TType_anon (anonInfo, tys) -> RecordAnonRecdInfo cenv anonInfo CheckTypesDeep cenv f g env tys diff --git a/tests/fsharp/Compiler/CompilerAssert.fs b/tests/fsharp/Compiler/CompilerAssert.fs index 50102ab21c4..1c753cd25e0 100644 --- a/tests/fsharp/Compiler/CompilerAssert.fs +++ b/tests/fsharp/Compiler/CompilerAssert.fs @@ -211,7 +211,7 @@ let main argv = 0""" typeCheckResults.Errors |> Array.distinctBy (fun e -> e.Severity, e.ErrorNumber, e.StartLineAlternate, e.StartColumn, e.EndLineAlternate, e.EndColumn, e.Message) - Assert.AreEqual(Array.length expectedTypeErrors, errors.Length, sprintf "Type check errors: %A" typeCheckResults.Errors) + Assert.AreEqual(Array.length expectedTypeErrors, errors.Length, sprintf "Type check errors: %A" errors) Array.zip errors expectedTypeErrors |> Array.iter (fun (info, expectedError) -> diff --git a/tests/fsharp/Compiler/Language/SpanTests.fs b/tests/fsharp/Compiler/Language/SpanTests.fs index cbac744a719..b1dc3c08a6f 100644 --- a/tests/fsharp/Compiler/Language/SpanTests.fs +++ b/tests/fsharp/Compiler/Language/SpanTests.fs @@ -3,6 +3,7 @@ namespace FSharp.Compiler.UnitTests open System +open FSharp.Compiler.SourceCodeServices open NUnit.Framework #if NETCOREAPP @@ -50,4 +51,55 @@ test () // We expect this error until System.Reflection.Emit gets fixed for emitting `modreq` on method calls. // See: https://github.com/dotnet/corefx/issues/29254 CompilerAssert.RunScript script [ "Method not found: '!0 ByRef System.ReadOnlySpan`1.get_Item(Int32)'." ] + + + [] + let ``Should fail to compile on a type abbreviation``() = + CompilerAssert.TypeCheckWithErrors """ +open System + +type Bytes = ReadOnlySpan + +type Test() = + + member _.M1 (data: Bytes) = + let x = + if false then + failwith "" + else + data + x + + member _.M2 (data: Bytes) = + let x = + if false then + failwithf "" + else + data + x + +let test () = + let span = ReadOnlySpan<_>(Array.empty) + let result = Test().M1(span) + let result = Test().M2(span) + 0 + """ + [| + FSharpErrorSeverity.Error, 412, (11, 17, 11, 28), "A type instantiation involves a byref type. This is not permitted by the rules of Common IL." + FSharpErrorSeverity.Error, 412, (19, 17, 19, 29), "A type instantiation involves a byref type. This is not permitted by the rules of Common IL." + FSharpErrorSeverity.Error, 412, (19, 27, 19, 29), "A type instantiation involves a byref type. This is not permitted by the rules of Common IL." + |] + + [] + let ``Should fail to compile on a type abbreviation 2``() = + CompilerAssert.TypeCheckWithErrors """ +open System + +type TA = Span * Span + +let f (x: TA) = () + """ + [| + FSharpErrorSeverity.Error, 3300, (6, 8, 6, 9), "The parameter 'x' has an invalid type 'TA'. This is not permitted by the rules of Common IL." + |] #endif \ No newline at end of file From e5d61cd84eb35caf7d8e107c7aabf3de2225598e Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 11 Dec 2019 00:16:29 -0800 Subject: [PATCH 3/6] Fixing build --- src/fsharp/PostInferenceChecks.fs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/fsharp/PostInferenceChecks.fs b/src/fsharp/PostInferenceChecks.fs index 7b62a1093bc..c5904dc2843 100644 --- a/src/fsharp/PostInferenceChecks.fs +++ b/src/fsharp/PostInferenceChecks.fs @@ -313,7 +313,7 @@ let RecordAnonRecdInfo cenv (anonInfo: AnonRecdTypeInfo) = // approx walk of type //-------------------------------------------------------------------------- -let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, visitTraitSolutionOpt, visitTyparOpt) as f) g env isInner ty = +let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, visitTraitSolutionOpt, visitTyparOpt) as f) (g: TcGlobals) env isInner ty = // We iterate the _solved_ constraints as well, to pick up any record of trait constraint solutions // This means we walk _all_ the constraints _everywhere_ in a type, including // those attached to _solved_ type variables. This is used by PostTypeCheckSemanticChecks to detect uses of @@ -332,7 +332,14 @@ let rec CheckTypeDeep (cenv: cenv) ((visitTy, visitTyconRefOpt, visitAppTyOpt, v | _ -> ()) | _ -> () - let ty = stripTyEqns g ty + let ty = + if g.compilingFslib then + match stripTyparEqns ty with + // When compiling FSharp.Core, do not strip type equations at this point if we can't dereference a tycon. + | TType_app (tcref, _) when not tcref.CanDeref -> ty + | _ -> stripTyEqns g ty + else + stripTyEqns g ty visitTy ty match ty with @@ -457,7 +464,7 @@ let CheckEscapes cenv allowProtected m syntacticArgs body = (* m is a range suit let AccessInternalsVisibleToAsInternal thisCompPath internalsVisibleToPaths access = // Each internalsVisibleToPath is a compPath for the internals of some assembly. // Replace those by the compPath for the internals of this assembly. - // This makes those internals visible here, but still internal. Bug://3737 + // This makes those internals visible here, but still internal. Bug://3737bui (access, internalsVisibleToPaths) ||> List.fold (fun access internalsVisibleToPath -> accessSubstPaths (thisCompPath, internalsVisibleToPath) access) From 4c4591232c066ea6d40da6d90f55111ca20c2025 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 11 Dec 2019 01:39:02 -0800 Subject: [PATCH 4/6] Update test --- tests/fsharp/typecheck/sigs/neg_byref_13.bsl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/fsharp/typecheck/sigs/neg_byref_13.bsl b/tests/fsharp/typecheck/sigs/neg_byref_13.bsl index 92efcae6d94..985177e9793 100644 --- a/tests/fsharp/typecheck/sigs/neg_byref_13.bsl +++ b/tests/fsharp/typecheck/sigs/neg_byref_13.bsl @@ -1,2 +1,8 @@ neg_byref_13.fs(2,6,2,8): typecheck error FS0445: The type abbreviation contains byrefs. This is not permitted by F#. + +neg_byref_13.fs(3,12,3,13): typecheck error FS3300: The parameter 'x' has an invalid type 'M5'. This is not permitted by the rules of Common IL. + +neg_byref_13.fs(3,5,3,10): typecheck error FS3301: The function or method has an invalid return type 'M5'. This is not permitted by the rules of Common IL. + +neg_byref_13.fs(3,20,3,21): typecheck error FS0412: A type instantiation involves a byref type. This is not permitted by the rules of Common IL. \ No newline at end of file From db7510aa7d42f36a04502bf21eb4ba3ee49bd955 Mon Sep 17 00:00:00 2001 From: "Kevin Ransom (msft)" Date: Sun, 15 Dec 2019 12:11:46 -0800 Subject: [PATCH 5/6] Apply suggestions from code review I like @cartermps test name changes. Co-Authored-By: Phillip Carter --- tests/fsharp/Compiler/Language/SpanTests.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/fsharp/Compiler/Language/SpanTests.fs b/tests/fsharp/Compiler/Language/SpanTests.fs index b1dc3c08a6f..646032880ac 100644 --- a/tests/fsharp/Compiler/Language/SpanTests.fs +++ b/tests/fsharp/Compiler/Language/SpanTests.fs @@ -54,7 +54,7 @@ test () [] - let ``Should fail to compile on a type abbreviation``() = + let ``Invalid usage of type abbreviated span should fail to compile``() = CompilerAssert.TypeCheckWithErrors """ open System @@ -91,7 +91,7 @@ let test () = |] [] - let ``Should fail to compile on a type abbreviation 2``() = + let ``Type abbreviation that boxes a span should fail to compile``() = CompilerAssert.TypeCheckWithErrors """ open System @@ -102,4 +102,4 @@ let f (x: TA) = () [| FSharpErrorSeverity.Error, 3300, (6, 8, 6, 9), "The parameter 'x' has an invalid type 'TA'. This is not permitted by the rules of Common IL." |] -#endif \ No newline at end of file +#endif From e30c4f2e0b21e7d96d35a5d2181f017419f7eb87 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Sun, 15 Dec 2019 14:59:19 -0800 Subject: [PATCH 6/6] Update PostInferenceChecks.fs --- src/fsharp/PostInferenceChecks.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsharp/PostInferenceChecks.fs b/src/fsharp/PostInferenceChecks.fs index c5904dc2843..1730f88bd2c 100644 --- a/src/fsharp/PostInferenceChecks.fs +++ b/src/fsharp/PostInferenceChecks.fs @@ -464,7 +464,7 @@ let CheckEscapes cenv allowProtected m syntacticArgs body = (* m is a range suit let AccessInternalsVisibleToAsInternal thisCompPath internalsVisibleToPaths access = // Each internalsVisibleToPath is a compPath for the internals of some assembly. // Replace those by the compPath for the internals of this assembly. - // This makes those internals visible here, but still internal. Bug://3737bui + // This makes those internals visible here, but still internal. Bug://3737 (access, internalsVisibleToPaths) ||> List.fold (fun access internalsVisibleToPath -> accessSubstPaths (thisCompPath, internalsVisibleToPath) access)