Skip to content
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

std/views (1st class openArrray, but not escape-safe); std/pointers; fixes sequtils.applyIt #14869

Closed
wants to merge 2 commits into from

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Jul 1, 2020

this might need to be split in 2 or 3 PRs (and I probably will add an RFC for std/views), but it shows how it can all fit together:

  • std/views: first class openArrays but not escape-safe, until we finally get escape-safe first class openArray (make openArray first-class RFCs#88) or some borrowing mechanism for this; I have some concrete idea for how to do this in a escape safe way with compiler support but it's more involved and a escape-unsafe version of it is useful in the meantime, in particular for performance reasons;

escape-safe

It's escape unsafe in the sense that the compiler won't give error when a stack address escapes its stack frame, other than that it's safe to use and solves actual problems. Later versions of the compiler can either offer a memory safe alternative or turn that type into one (and prevent stack address escaping).

View[T] vs UncheckedArray[T]

View[T] is in particular safer and much more useful than UncheckedArray since the length is a property; for example it allows items/mitems, as well as hosts of API's (whereas UncheckedArray is just a disguised pointer that doesn't know it's valid range, so cannot be composed with other API's).

naming

previous name in this PR was std/chunks+Chunk[T]/MChunk[T], but then I thought about View and it's much better fit. Other names I considered:

  • slices+Slice[T]+MSlice[T] would be a good name but system.Slice already exists
  • span+Span[T]+MSpan[T] (as in C#) is also possible

StringView

I will also add StringView which will be built on top of View[T]

std/pointers

std/pointers could also be split into its own PR; it allows a clean syntax for pointer indexing and I find it useful in many contexts

applyIt is now correct and more efficient

applyIt is now made correct with respect to side effects, while at the same time avoiding un-needed copies, both with non-openArray and openArray types. This is made possible via evalonceVar. See tests. Note that I couldn't use evalOnceAs because I needed a var, not let

evalOnceAs (and other procs in sequtils) can now be side-effect safe even for openArray inputs

evalOnceAs had a caveat: it didn't work for openArray; with View[T], this can now be made correct wrt double evaluation, using same approach as I did for evalonceVar

var s {.evalonce.} = varSeq

right now I had to introduce evalonceVar, and I had to use it as evalonceVar(a, typ, expr), but in future it can be used as var s {.evalonce.} = varSeq pending fixing those:

@timotheecour timotheecour changed the title std/pointers; std/chunks; fixes sequtils.applyIt std/chunks (memory unsafe but 1st class openArrray); std/pointers; fixes sequtils.applyIt Jul 1, 2020
@timotheecour timotheecour mentioned this pull request Jul 1, 2020
@juancarlospaco
Copy link
Collaborator

Maybe a name more descriptive than "chunks" 🤔

@timotheecour
Copy link
Member Author

I don't like chunks too much either, but please suggest something... as described, slice would be the ideal (Slice+MSlice) name but it's already taken by system.Slice. I'd like something:

  • short (ideally single workd, not compound one)
  • that can be prefixed with M (like Chunk+MChunk)

timotheecour added a commit to timotheecour/ginger that referenced this pull request Jul 1, 2020
@RSDuck
Copy link
Contributor

RSDuck commented Jul 2, 2020

how about Window?

EDIT: noticed, the plural is bad

@alaviss
Copy link
Collaborator

alaviss commented Jul 2, 2020

How about View?

@timotheecour timotheecour changed the title std/chunks (memory unsafe but 1st class openArrray); std/pointers; fixes sequtils.applyIt std/views (1st class openArrray, but not escape-safe); std/pointers; fixes sequtils.applyIt Jul 2, 2020
@timotheecour
Copy link
Member Author

@alaviss lol, you read my mind, I've already pushed a commit that changes everything to std/view+View[T]+MView[T] (and expanded on the API; it's not the final version yet)

@Varriount
Copy link
Contributor

Varriount commented Jul 2, 2020

I don't really see the point of introducing this into the standard library if a better version can be implemented instead. If people need it that badly, is rather see it as a nimble package. Once something is included in the standard library, it's difficult to remove.

@juancarlospaco
Copy link
Collaborator

I like the name views; I think proc like these for pointers in stdlib been rejected in the past.

@alaviss
Copy link
Collaborator

alaviss commented Jul 3, 2020

This is a good candidate for fusion if you don't mind :)

@Araq
Copy link
Member

Araq commented Oct 27, 2020

We got --experimental:views instead which was covered by an RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants