-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
borrow checking #15282
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
borrow checking #15282
Conversation
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.
I can't comment on the implementation but we'll likely want to have an easy to approach API and story.
openArray
is long to type and very obscure apart from Oberon, Modula and Pascal devs, I would love to see it replaced either by view or span (given that slice is taken). C# and C++ also use span and call views the overarching type category that doesn't own memory just like current http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0122r1.pdf . (Rust and Swift uses Slice)toOpenArray
should be using[]
except when the compiler detects that the assignment escapes (which it now can do with borrow checking, write-tracking https://nim-lang.org/blog/2020/09/01/write-tracking.html. This has been a source of slow allocation and GC pressures in Nimbus (avoid unnecessary seq allocations status-im/nimbus-eth2#1573) and is an easy way to get in trouble with multithreading especially OpenMP when we forget to setupForeignThreadGc- We probably need a StringView type
I should probably write a RFC on that.
echo a.len | ||
|
||
proc main(s: seq[int]) = | ||
var x: openArray[int] = s # 'x' is a view into 's' |
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.
I think a var x = s.view()
or var x = s.borrow()
would be nicer but I guess we can discuss API once the low-level primitive work.
echo x[i] | ||
take(x) | ||
|
||
take(x.toOpenArray(0, 1)) # slicing remains possible |
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.
Similarly []
on sequences should now return openArray by default unless this is assigned to a returned value that escapes and maybe we need a StringView
type?
Well I won't rename or introduce new APIs, I'm following nim-lang/RFCs#178 If you want new names or APIs please write an RFC and see how the community reacts. ;-) |
this is so cool and pretty much the only thing I felt was missing from Nim! While this is going to be experimental at first, I think it deprecates multiple things which were more or less workarounds using pointers:
|
Agreed. Also the special casing for |
Merging this as the dangers of merge conflicts become serious. But it's hardly usable in this state and will see follow-up PRs. |
* refactoring: move procs to typeallowed.nim * frontend preparations for first class openArray support * prepare the code generator for first class openArray * code generation for first class openArray; WIP * code generation for open arrays, progress * added isViewType proc * preparations for borrow checking * added borrow checking to the front end
Todo: