-
Notifications
You must be signed in to change notification settings - Fork 844
Ref/Span fixes #5146
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
Ref/Span fixes #5146
Conversation
1d915c6 to
ae5e5a8
Compare
src/fsharp/PostInferenceChecks.fs
Outdated
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.
This code is a bit complicated, but something like this is necessary when we start supporting stack referring span likes, and mix call functions with arguments that have both stack + heap referring span likes.
src/fsharp/PostInferenceChecks.fs
Outdated
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.
Hmm, at this point I may want to return a LimitValFlags.ByRef here.
|
I'm using these as tests: As of this commit: 1359ca7 let mutable beef = 1
let returning_span (x: byref<int>) = Span.Empty
let returning_ref (x: byref<int>) = &x
let returning_ref2 (x: byref<int>) (y: byref<int>) = &x
let should_not_work1 () =
let mutable x = 1
&x
let should_not_work2 () =
let mutable x = 1
let mutable x = (let mutable y = &x in &y)
let y = &returning_ref &x
&y
let should_not_work3 () =
let mutable x = 1
let mutable x =
match &x with
| x -> &x
let y = &returning_ref &x
&y
let should_not_work4 () =
let mutable x = 1
let mutable y = &x
y <- 1
&y
let should_not_work5 () =
let x = 1
let y = &x
&y
let should_not_work6 () =
let mutable x = 1
&returning_ref &x
let should_not_work7 () =
let mutable x = 1
&returning_ref2 &beef &x
let should_not_work8 () =
let mutable x = 1
let y = &returning_ref2 &beef &x
&y
let should_work1 () =
Span.Empty
let should_work3 () =
let y = &returning_ref &beef
&y
let should_work2 () =
let x = Span.Empty
x
let should_work4 () =
&returning_ref2 &beef &beef
let should_work5 () =
let mutable x = 1
returning_ref2 &x &x
let should_work6 () =
let mutable x = 1
returning_span &x
let should_work7 () =
let mutable x = 1
let y = returning_span &x
yAll behave as intended except for "should_not_work2" and "should_not_work3", which pass in the compiler. Will need to dig into the ast a bit more. |
|
Tagging as 15.8 - we're in escrow for Preview 3, so ideally this would get in there, but we may have to get it into Preview 4 instead. Which is probably fine, it would just mean that we'd have a set of known issues for Span in Preview 3. |
|
Here are some updated cases: let mutable beef = 1
let returning_span (x: byref<int>) = Span.Empty
let returning_ref (x: byref<int>) = &x
let returning_ref2 (x: byref<int>) (y: byref<int>) = &x
let passing_span_returning_span (x: Span<int>) = Span.Empty
let passing_span_returnin_ref (x: Span<int>) = &x.[0]
let passing_span2 (x: Span<int>) (y: Span<int>) = ()
let passing_byref_of_span_returning_span (x: byref<Span<int>>) = x
let passing_byref_of_span_returning_byref_of_span (x: byref<Span<int>>) = &x
let should_not_work () =
let test = ReadOnlySpan.Empty
let f = fun () -> test
()
let should_not_work1 () =
let mutable x = 1
&x
let should_not_work2 () =
let mutable x = 1
let mutable x = (let mutable y = &x in &y)
let y = &returning_ref &x
&y
let should_not_work3 () =
let mutable x = 1
let mutable x =
match &x with
| x -> &x
let y = &returning_ref &x
&y
let should_not_work4 () =
let mutable x = 1
let mutable y = &x
y <- 1
&y
let should_not_work5 () =
let x = 1
let y = &x
&y
let should_not_work6 () =
let mutable x = 1
&returning_ref &x
let should_not_work7 () =
let mutable x = 1
&returning_ref2 &beef &x
let should_not_work8 () =
let mutable x = 1
let y = &returning_ref2 &beef &x
&y
let should_not_work9 () =
let mutable s = Span.Empty
passing_span_returning_span s
let should_not_work10 () =
let mutable s = Span.Empty
&passing_span_returnin_ref s
let should_not_work11 () =
let mutable s = Span.Empty
&passing_span_returnin_ref (passing_span_returning_span s)
let should_not_work12 () =
let mutable s = Span.Empty
passing_byref_of_span_returning_span &s
let should_not_work13 () =
let mutable s = Span.Empty
s
let should_not_work14 () =
let mutable s = Span.Empty
let mutable x = &s
passing_byref_of_span_returning_span &x
let should_work1 () =
Span.Empty
let should_work3 () =
let y = &returning_ref &beef
&y
let should_work2 () =
let x = Span.Empty
x
let should_work4 () =
&returning_ref2 &beef &beef
let should_work5 () =
let mutable x = 1
returning_ref2 &x &x
let should_work6 () =
let mutable x = 1
returning_span &x
let should_work7 () =
let mutable x = 1
let y = returning_span &x
y
let should_work8 () =
let s = Span.Empty
passing_span_returning_span s
let should_work9 () =
let mutable s = Span.Empty
passing_span_returnin_ref s
let should_work10 () =
let mutable s = Span.Empty
passing_span_returnin_ref (passing_span_returning_span s)
let should_work11 () =
let s = Span.Empty
&passing_span_returnin_ref (passing_span_returning_span s)
let should_work12 () =
let s = Span.Empty
passing_byref_of_span_returning_span &s
let should_work13 () =
let s = Span.Empty
let mutable x = &s
passing_byref_of_span_returning_span &x
let should_work14 (s: byref<Span<int>>) =
passing_byref_of_span_returning_byref_of_span &s
let should_work15 (s: byref<Span<int>>) =
let x = &s
let y = &passing_byref_of_span_returning_byref_of_span &x
&y
type TestDelegate = delegate of byref<Span<int>> -> Span<int>
let should_work16 (s: byref<Span<int>>) =
let f = TestDelegate(fun _ -> Span.Empty)
//let mutable s = Span.Empty
f.Invoke(&s)
type IBeef =
abstract Test : byref<Span<int>> -> byref<Span<int>>
let should_work17 () =
{ new IBeef with
member __.Test s = &s
}
|
|
All tests are passing, this is a good sign. I will add more tests, code cleanup and comments. |
|
Fantastic work! |
|
I'm going to write more tests. Going to port some roslyn ones over. |
|
This is starting to become an epic - shouldn't we try to get it into the bank now and then you can send new PRs on top? |
|
We set a plan to get this in next week, hopefully Monday if all goes well. All that remains are a few more tests and I think it should be good to go. |
|
test Ubuntu16.04 Release_fcs Build please |
|
test Windows_NT Release_fcs Build please |
…able structs not having any rules for span likes. Updated a few error msgs. Added lots of tests.
…ious commit. Updated baselines
|
test Windows_NT Release_ci_part4 Build please |
|
Yay
Will Smith <notifications@github.com> schrieb am Mi., 20. Juni 2018, 21:13:
… Merged #5146 <#5146>.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5146 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADgNFcOP731Sz6AFsgPrEkni37O1HBGks5t-p7agaJpZM4Ugqns>
.
|
* Added LimitedValFlags for PostInferenceChecks * More work * Renamed IsLimited to IsLimitedSpanLike * Don't use isByRefReturnCall in CheckExprs * Minor cleanup * Changed error message. Fixed safety rule regarding byrefs returning and passing. * Fixed an edge case with returning span. Simplified Expr.App * Cleanup * Fixed a few ref return checks. Partially handling byref of span-like types * Simplifying flags. Added CheckCall * Updated test baseline. Added several CheckCall functions * Updated baseline tests * Handling byref of span likes * Added a little bit better error msg for LByrefGet * Removed commented code * Fixed test * Fixed inability to use byref<Span<>> for delegates and slots * Minor cleanup with some comments * Few more comments * Running neg tests in core/span. Fixed bug in IsByRefLike structs with span fields * Updated baseline * Fixed more issues with slots. Simplified PermitByRefType * Added a little better error messages on spans * Fixed record construction for span likes. Fixed setting fields on mutable structs not having any rules for span likes. Updated a few error msgs. Added lots of tests. * Updated error msg on function call. Added a removed comment from previous commit. Updated baselines * Added internalTestSpanStackReferring test option * Added special handling for receivers * Fixing build * Fixing build * Fixing build * More tests, updated baseline
* Ref/Span fixes (#5146) * Added LimitedValFlags for PostInferenceChecks * More work * Renamed IsLimited to IsLimitedSpanLike * Don't use isByRefReturnCall in CheckExprs * Minor cleanup * Changed error message. Fixed safety rule regarding byrefs returning and passing. * Fixed an edge case with returning span. Simplified Expr.App * Cleanup * Fixed a few ref return checks. Partially handling byref of span-like types * Simplifying flags. Added CheckCall * Updated test baseline. Added several CheckCall functions * Updated baseline tests * Handling byref of span likes * Added a little bit better error msg for LByrefGet * Removed commented code * Fixed test * Fixed inability to use byref<Span<>> for delegates and slots * Minor cleanup with some comments * Few more comments * Running neg tests in core/span. Fixed bug in IsByRefLike structs with span fields * Updated baseline * Fixed more issues with slots. Simplified PermitByRefType * Added a little better error messages on spans * Fixed record construction for span likes. Fixed setting fields on mutable structs not having any rules for span likes. Updated a few error msgs. Added lots of tests. * Updated error msg on function call. Added a removed comment from previous commit. Updated baselines * Added internalTestSpanStackReferring test option * Added special handling for receivers * Fixing build * Fixing build * Fixing build * More tests, updated baseline * Opening System in PostInferenceChecks
This is intended to fix several safety rules with ref returns and span.