-
Notifications
You must be signed in to change notification settings - Fork 847
[WIP] Test cases and minor fixes for "Fix inlining on subtypes" #839
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
Changes from all commits
02307b1
1cbb8ab
7551486
de75506
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
||
|
|
@@ -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; | ||
|
|
@@ -3055,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)) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure that this only happens when |
||
| #if DEBUG | ||
| if verboseOptimizations then dprintf "val %s gets opt info %s\n" (showL(valL v)) (showL(exprValueInfoL cenv.g einfo.Info)); | ||
| #endif | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| FSharp_* | ||
| FSharpQA_* | ||
| CoreUnit_* | ||
| TestResults | ||
| TestResults | ||
|
|
||
| **/_perm.txt |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test case looks the same as https://github.com/Microsoft/visualfsharp/pull/763/files#diff-e2959d5dc5cc275246c938d409549ee1R1699, which is added as a succeeding test case. Have you checked that this test case still gives the errors shown after the fix is made? |
||
| 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 = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I did not find out yet why the error is reported twice. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| namespace Neg01 | ||
|
|
||
| (* 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this change for and which test case covers it?