Skip to content

unblock https://github.com/nim-lang/Nim/pull/14887#21

Open
timotheecour wants to merge 1 commit into
vegansk:masterfrom
timotheecour:patch-1
Open

unblock https://github.com/nim-lang/Nim/pull/14887#21
timotheecour wants to merge 1 commit into
vegansk:masterfrom
timotheecour:patch-1

Conversation

@timotheecour
Copy link
Copy Markdown

refs nim-lang/Nim#14887
closes #20

@nigredo-tori
Copy link
Copy Markdown
Collaborator

nigredo-tori commented Oct 11, 2020

ospath/os thing is only the first step. We haven't updated the library for quite a while, and we will need a lot of changes to make it compatible with Nim 0.20 and later. I've done the easy parts here. This breaks the tests for Nim 0.19 (some issue with toSeq), but that can be fixed separately using when - or we can just drop 0.19 support.

One major issue I've found is that callback= in asyncfutures now requires a {.gcsafe.} function, which means that any combinators dealing with Future must only accept gcsafe procs as parameters. I've fixed the types for now, but that still means that the combinators are incompatible with sugar lambdas. ☹️

Also, spawn now doesn't accept closure procs. We will need to change concurrent accordingly, but I've ran out of steam by this point. @vegansk, could you please continue this work? You are more familiar with what's going on in that file.

@timotheecour
Copy link
Copy Markdown
Author

ospath/os thing is only the first step

but it's the only step needed to unblock nim-lang/Nim#14887, all other libraries mentioned there were patched :)

or we can just drop 0.19 support.

only supporting nim >= 1.0 makes sense IMO

now requires a {.gcsafe.} function

see {.gcsafe.}: to force gcsafe when you know it's safe to do so

@nigredo-tori
Copy link
Copy Markdown
Collaborator

see {.gcsafe.}: to force gcsafe when you know it's safe to do so

This works, and this is what I did in my branch, but it's not compatible with the -> and => syntax from sugar, which hurts usability. Still, there's not much we can do about this, since it's enforced by Future itself.

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.

remove deprecated import ospaths (blocks https://github.com/nim-lang/Nim/pull/14887)

2 participants