From 02307b150ff7a6a04917e5562402fb3ae053f743 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 4 Dec 2015 17:50:48 +0000 Subject: [PATCH 1/3] Fix inlining on subtypes --- src/fsharp/Optimizer.fs | 9 ++++++++- tests/fsharp/core/subtype/test.fsx | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/fsharp/Optimizer.fs b/src/fsharp/Optimizer.fs index ec0c4d6686f..c22d661df44 100644 --- a/src/fsharp/Optimizer.fs +++ b/src/fsharp/Optimizer.fs @@ -2749,7 +2749,14 @@ and OptimizeLambdas (vspec: Val option) cenv env topValInfo e ety = let valu = match baseValOpt with | None -> CurriedLambdaValue (lambdaId,arities,bsize,expr',ety) - | _ -> UnknownValue + | Some baseVal -> + let fvs = freeInExpr CollectLocals body' + if fvs.UsesMethodLocalConstructs || fvs.FreeLocals.Contains baseVal then + UnknownValue + else + let expr2 = mkMemberLambdas m tps ctorThisValOpt None vsl (body',bodyty) + CurriedLambdaValue (lambdaId,arities,bsize,expr2,ety) + expr', { TotalSize=bsize + (if isTopLevel then methodDefnTotalSize else closureTotalSize); (* estimate size of new syntactic closure - expensive, in contrast to a method *) FunctionSize=1; diff --git a/tests/fsharp/core/subtype/test.fsx b/tests/fsharp/core/subtype/test.fsx index 321c202c963..4b594f3f4c1 100644 --- a/tests/fsharp/core/subtype/test.fsx +++ b/tests/fsharp/core/subtype/test.fsx @@ -1696,6 +1696,25 @@ module RecordPropertyConstraintTests = check "ckjwnewk" (f8()) (System.TimeSpan.FromSeconds 2.0) // after mutation check "ckjwnewk" (f10()) "Gary" +// See https://github.com/Microsoft/visualfsharp/issues/740 - inlining on subtypes was not allowed +module InliningOnSubTypes1 = + type A() = + static member inline dosomething() = () + + type B() = + inherit A() + member inline this.SomethingElse a = a + 10 + member inline this.SomethingElse2 a b = a + b + 10 + + let f () = + let b = B() + let x1 = b.SomethingElse 3 + let x2 = b.SomethingElse2 3 4 + (x1, x2) + do check "clkewlijwlkw" (f()) (13, 17) + + + // See https://github.com/Microsoft/visualfsharp/issues/238 module GenericPropertyConstraintSolvedByRecord = From 75514862c46ba05bf94fc6f9c175877072ff859c Mon Sep 17 00:00:00 2001 From: mpetruska Date: Sat, 26 Dec 2015 00:28:46 +0000 Subject: [PATCH 2/3] adds test case for inlined member that uses base reference --- tests/.gitignore | 4 +++- tests/fsharp/typecheck/.gitignore | 6 ++++-- tests/fsharp/typecheck/sigs/build.bat | 3 +++ tests/fsharp/typecheck/sigs/neg95.bsl | 2 ++ tests/fsharp/typecheck/sigs/neg95.fs | 17 +++++++++++++++++ tests/fsharp/typecheck/tests_typecheck.fs | 3 ++- 6 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 tests/fsharp/typecheck/sigs/neg95.bsl create mode 100644 tests/fsharp/typecheck/sigs/neg95.fs diff --git a/tests/.gitignore b/tests/.gitignore index eca5ba1118f..531297fba98 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -1,4 +1,6 @@ FSharp_* FSharpQA_* CoreUnit_* -TestResults \ No newline at end of file +TestResults + +**/_perm.txt \ No newline at end of file diff --git a/tests/fsharp/typecheck/.gitignore b/tests/fsharp/typecheck/.gitignore index 4bfefa6abed..68d1a2ceee0 100644 --- a/tests/fsharp/typecheck/.gitignore +++ b/tests/fsharp/typecheck/.gitignore @@ -1,7 +1,9 @@ sigs/*.err sigs/*.vserr -sigs/pos*.dll -sigs/pos*.exe +sigs/*.dll +sigs/*.exe +sigs/*.diff +sigs/*.vsdiff full-rank-arrays/HighRankArrayTests.dll diff --git a/tests/fsharp/typecheck/sigs/build.bat b/tests/fsharp/typecheck/sigs/build.bat index 9e4f4f00778..75b47a64bf3 100644 --- a/tests/fsharp/typecheck/sigs/build.bat +++ b/tests/fsharp/typecheck/sigs/build.bat @@ -308,6 +308,9 @@ call ..\..\single-neg-test.bat neg41 call ..\..\single-neg-test.bat neg42 @if ERRORLEVEL 1 goto Error +call ..\..\single-neg-test.bat neg95 +@if ERRORLEVEL 1 goto Error + "%FSC%" %fsc_flags% -a -o:pos07.dll pos07.fs @if ERRORLEVEL 1 goto Error diff --git a/tests/fsharp/typecheck/sigs/neg95.bsl b/tests/fsharp/typecheck/sigs/neg95.bsl new file mode 100644 index 00000000000..5264fc047f2 --- /dev/null +++ b/tests/fsharp/typecheck/sigs/neg95.bsl @@ -0,0 +1,2 @@ + +neg95.fs(16,22): error FS1113: The value 's2' was marked inline but its implementation makes use of an internal or private function which is not sufficiently accessible diff --git a/tests/fsharp/typecheck/sigs/neg95.fs b/tests/fsharp/typecheck/sigs/neg95.fs new file mode 100644 index 00000000000..e55f64e64ab --- /dev/null +++ b/tests/fsharp/typecheck/sigs/neg95.fs @@ -0,0 +1,17 @@ +namespace Neg95 + +(* Optimizer should not throw InternalError when it + encounters an inline method that references `base`, + a compilation error should be reported instead. + See issue: https://github.com/Microsoft/visualfsharp/issues/740 *) + +type Base<'T>() = + + member __.s() = () + +type Class<'T>() = + inherit Base<'T>() + + member inline __.Bar x = x + member inline __.s2() = base.s() + member inline this.s3() = this.Bar 1 |> ignore diff --git a/tests/fsharp/typecheck/tests_typecheck.fs b/tests/fsharp/typecheck/tests_typecheck.fs index fde38747bf0..9fb6ea0024e 100644 --- a/tests/fsharp/typecheck/tests_typecheck.fs +++ b/tests/fsharp/typecheck/tests_typecheck.fs @@ -196,7 +196,8 @@ module Sigs = // call ..\..\single-neg-test.bat neg40 // call ..\..\single-neg-test.bat neg41 // call ..\..\single-neg-test.bat neg42 - do! processor.For (["neg87"; "neg86"; "neg85"; "neg84"; "neg83"; "neg82"; "neg81"; "neg80"; "neg79"; "neg78"; "neg77"; "neg76"; "neg75"; "neg74"; "neg73"; "neg72"; "neg71"; "neg70"; "neg69"; "neg68"; "neg67"; "neg66"; "neg65"; "neg64"; "neg61"; "neg63"; "neg62"; "neg20"; "neg24"; "neg32"; "neg37"; "neg37_a"; "neg60"; "neg59"; "neg58"; "neg57"; "neg56"; "neg56_a"; "neg56_b"; "neg55"; "neg54"; "neg53"; "neg52"; "neg51"; "neg50"; "neg49"; "neg48"; "neg47"; "neg46"; "neg10"; "neg10_a"; "neg45"; "neg44"; "neg43"; "neg38"; "neg39"; "neg40"; "neg41"; "neg42"], singleNegTest) + // call ..\..\single-neg-test.bat neg95 + do! processor.For (["neg87"; "neg86"; "neg85"; "neg84"; "neg83"; "neg82"; "neg81"; "neg80"; "neg79"; "neg78"; "neg77"; "neg76"; "neg75"; "neg74"; "neg73"; "neg72"; "neg71"; "neg70"; "neg69"; "neg68"; "neg67"; "neg66"; "neg65"; "neg64"; "neg61"; "neg63"; "neg62"; "neg20"; "neg24"; "neg32"; "neg37"; "neg37_a"; "neg60"; "neg59"; "neg58"; "neg57"; "neg56"; "neg56_a"; "neg56_b"; "neg55"; "neg54"; "neg53"; "neg52"; "neg51"; "neg50"; "neg49"; "neg48"; "neg47"; "neg46"; "neg10"; "neg10_a"; "neg45"; "neg44"; "neg43"; "neg38"; "neg39"; "neg40"; "neg41"; "neg42"; "neg95"], singleNegTest) // "%FSC%" %fsc_flags% -a -o:pos07.dll pos07.fs do! fsc "%s -a -o:pos07.dll" fsc_flags ["pos07.fs"] From de75506e09f0abe866315e95de195c824aade0f3 Mon Sep 17 00:00:00 2001 From: mpetruska Date: Sat, 26 Dec 2015 01:42:57 +0000 Subject: [PATCH 3/3] adds test case, fixes additional issue with base refs in inline method --- src/fsharp/Optimizer.fs | 4 ++-- tests/fsharp/optimize/.gitignore | 2 ++ tests/fsharp/optimize/inline/build.bat | 3 +++ tests/fsharp/optimize/inline/neg01.bsl | 4 ++++ .../{typecheck/sigs/neg95.fs => optimize/inline/neg01.fs} | 2 +- tests/fsharp/typecheck/sigs/build.bat | 3 --- tests/fsharp/typecheck/sigs/neg95.bsl | 2 -- tests/fsharp/typecheck/tests_typecheck.fs | 3 +-- 8 files changed, 13 insertions(+), 10 deletions(-) create mode 100644 tests/fsharp/optimize/inline/neg01.bsl rename tests/fsharp/{typecheck/sigs/neg95.fs => optimize/inline/neg01.fs} (96%) delete mode 100644 tests/fsharp/typecheck/sigs/neg95.bsl diff --git a/src/fsharp/Optimizer.fs b/src/fsharp/Optimizer.fs index c22d661df44..16bfa69bd3d 100644 --- a/src/fsharp/Optimizer.fs +++ b/src/fsharp/Optimizer.fs @@ -1066,7 +1066,7 @@ let AbstractLazyModulInfoByHiding isAssemblyBoundary mhi = ValInfos = ValInfos(ss.ValInfos.Entries |> Seq.filter (fun (vref,_) -> not (hiddenVal vref.Deref)) - |> Seq.map (fun (vref,e) -> check (* "its implementation uses a binding hidden by a signature" m *) vref (abstractValInfo e) )) } + |> Seq.map (fun (vref,e) -> (vref, (abstractValInfo e) ))) } and abstractLazyModulInfo (ss:LazyModuleInfo) = ss.Force() |> abstractModulInfo |> notlazy @@ -3062,7 +3062,7 @@ and OptimizeBinding cenv isRec env (TBind(v,e,spBind)) = then {einfo with Info=UnknownValue} else einfo if v.MustInline && IsPartialExprVal einfo.Info then - errorR(InternalError("the mustinline value '"^v.LogicalName^"' was not inferred to have a known value",v.Range)); + errorR(NumberedError(FSComp.SR.optValueMarkedInlineButIncomplete(v.DisplayName), v.Range)) #if DEBUG if verboseOptimizations then dprintf "val %s gets opt info %s\n" (showL(valL v)) (showL(exprValueInfoL cenv.g einfo.Info)); #endif diff --git a/tests/fsharp/optimize/.gitignore b/tests/fsharp/optimize/.gitignore index 2de025a1737..1ec792c9c34 100644 --- a/tests/fsharp/optimize/.gitignore +++ b/tests/fsharp/optimize/.gitignore @@ -8,6 +8,8 @@ inline/FSharpOptimizationData.* inline/FSharpSignatureData.* inline/*.il inline/*.res +inline/*.err +inline/*.diff stats/*.resources stats/*.il diff --git a/tests/fsharp/optimize/inline/build.bat b/tests/fsharp/optimize/inline/build.bat index a75d70755fa..4745a80026f 100644 --- a/tests/fsharp/optimize/inline/build.bat +++ b/tests/fsharp/optimize/inline/build.bat @@ -28,6 +28,9 @@ if ERRORLEVEL 1 goto Error "%FSC%" %fsc_flags% --optimize -o:test--optimize.exe -g test.fs -r:lib--optimize.dll -r:lib3--optimize.dll if ERRORLEVEL 1 goto Error +call ..\..\single-neg-test.bat neg01 +@if ERRORLEVEL 1 goto Error + :Ok echo Built fsharp %~f0 ok. echo. > build.ok diff --git a/tests/fsharp/optimize/inline/neg01.bsl b/tests/fsharp/optimize/inline/neg01.bsl new file mode 100644 index 00000000000..529c3e96a77 --- /dev/null +++ b/tests/fsharp/optimize/inline/neg01.bsl @@ -0,0 +1,4 @@ + +neg01.fs(16,22,16,24): optimize error FS1113: The value 's2' was marked inline but its implementation makes use of an internal or private function which is not sufficiently accessible + +neg01.fs(16,22,16,24): optimize error FS1113: The value 's2' was marked inline but its implementation makes use of an internal or private function which is not sufficiently accessible diff --git a/tests/fsharp/typecheck/sigs/neg95.fs b/tests/fsharp/optimize/inline/neg01.fs similarity index 96% rename from tests/fsharp/typecheck/sigs/neg95.fs rename to tests/fsharp/optimize/inline/neg01.fs index e55f64e64ab..9953715a42c 100644 --- a/tests/fsharp/typecheck/sigs/neg95.fs +++ b/tests/fsharp/optimize/inline/neg01.fs @@ -1,4 +1,4 @@ -namespace Neg95 +namespace Neg01 (* Optimizer should not throw InternalError when it encounters an inline method that references `base`, diff --git a/tests/fsharp/typecheck/sigs/build.bat b/tests/fsharp/typecheck/sigs/build.bat index 75b47a64bf3..9e4f4f00778 100644 --- a/tests/fsharp/typecheck/sigs/build.bat +++ b/tests/fsharp/typecheck/sigs/build.bat @@ -308,9 +308,6 @@ call ..\..\single-neg-test.bat neg41 call ..\..\single-neg-test.bat neg42 @if ERRORLEVEL 1 goto Error -call ..\..\single-neg-test.bat neg95 -@if ERRORLEVEL 1 goto Error - "%FSC%" %fsc_flags% -a -o:pos07.dll pos07.fs @if ERRORLEVEL 1 goto Error diff --git a/tests/fsharp/typecheck/sigs/neg95.bsl b/tests/fsharp/typecheck/sigs/neg95.bsl deleted file mode 100644 index 5264fc047f2..00000000000 --- a/tests/fsharp/typecheck/sigs/neg95.bsl +++ /dev/null @@ -1,2 +0,0 @@ - -neg95.fs(16,22): error FS1113: The value 's2' was marked inline but its implementation makes use of an internal or private function which is not sufficiently accessible diff --git a/tests/fsharp/typecheck/tests_typecheck.fs b/tests/fsharp/typecheck/tests_typecheck.fs index 9fb6ea0024e..fde38747bf0 100644 --- a/tests/fsharp/typecheck/tests_typecheck.fs +++ b/tests/fsharp/typecheck/tests_typecheck.fs @@ -196,8 +196,7 @@ module Sigs = // call ..\..\single-neg-test.bat neg40 // call ..\..\single-neg-test.bat neg41 // call ..\..\single-neg-test.bat neg42 - // call ..\..\single-neg-test.bat neg95 - do! processor.For (["neg87"; "neg86"; "neg85"; "neg84"; "neg83"; "neg82"; "neg81"; "neg80"; "neg79"; "neg78"; "neg77"; "neg76"; "neg75"; "neg74"; "neg73"; "neg72"; "neg71"; "neg70"; "neg69"; "neg68"; "neg67"; "neg66"; "neg65"; "neg64"; "neg61"; "neg63"; "neg62"; "neg20"; "neg24"; "neg32"; "neg37"; "neg37_a"; "neg60"; "neg59"; "neg58"; "neg57"; "neg56"; "neg56_a"; "neg56_b"; "neg55"; "neg54"; "neg53"; "neg52"; "neg51"; "neg50"; "neg49"; "neg48"; "neg47"; "neg46"; "neg10"; "neg10_a"; "neg45"; "neg44"; "neg43"; "neg38"; "neg39"; "neg40"; "neg41"; "neg42"; "neg95"], singleNegTest) + do! processor.For (["neg87"; "neg86"; "neg85"; "neg84"; "neg83"; "neg82"; "neg81"; "neg80"; "neg79"; "neg78"; "neg77"; "neg76"; "neg75"; "neg74"; "neg73"; "neg72"; "neg71"; "neg70"; "neg69"; "neg68"; "neg67"; "neg66"; "neg65"; "neg64"; "neg61"; "neg63"; "neg62"; "neg20"; "neg24"; "neg32"; "neg37"; "neg37_a"; "neg60"; "neg59"; "neg58"; "neg57"; "neg56"; "neg56_a"; "neg56_b"; "neg55"; "neg54"; "neg53"; "neg52"; "neg51"; "neg50"; "neg49"; "neg48"; "neg47"; "neg46"; "neg10"; "neg10_a"; "neg45"; "neg44"; "neg43"; "neg38"; "neg39"; "neg40"; "neg41"; "neg42"], singleNegTest) // "%FSC%" %fsc_flags% -a -o:pos07.dll pos07.fs do! fsc "%s -a -o:pos07.dll" fsc_flags ["pos07.fs"]