make @sync lexically scoped and merge @schedule with @async#27164
make @sync lexically scoped and merge @schedule with @async#27164JeffBezanson merged 1 commit intomasterfrom
@sync lexically scoped and merge @schedule with @async#27164Conversation
vchuravy
left a comment
There was a problem hiding this comment.
My one comment would be that it is easier to deprecate @async since that is the behaviour this changes. But in general I don't mind much and I think that @async is the better name.
@sync lexically scoped and merge @schedule with @async@sync lexically scoped and merge @schedule with @async
NEWS.md
Outdated
| * `mv`,`cp`, `touch`, `mkdir`, `mkpath` now return the path that was created/modified | ||
| rather than `nothing` ([#27071]). | ||
|
|
||
| * `@sync` now waits only for *lexically* enclosed (i.e. visible directly in the source |
There was a problem hiding this comment.
Probably worth mentioning the deprecation of @schedule, no?
|
I'm a little worried by the removal of Some piece of code might call the new Another potential pitfall is a small function that starts out with a I can sympathise with @JeffBezanson's desire to remove unneeded confusion between
Another possibility would be to remove |
|
I think that's an important concern. Bringing back schedule but keeping the new behaviour for async seems like a good idea to me. |
With the upcoming threading runtime we will likely be adding more parallel constructs. Unfortunately, we have a few too many similar things (
@async,@schedule,@threads,@spawn, ...). Confusion between@asyncand@scheduleis particularly common and unnecessary; for example some code in the Sockets stdlib uses@asyncwhere it should have used@schedule.This change improves the situation by removing
@schedule. The key change to make this possible is to make@synclexically scoped. That way, if an@asyncappears on its own it just spawns a task and nothing else happens. But if it's lexically surrounded by a@sync, then that sync waits for all the async stuff inside.It looks to me like basically all uses of
@syncin the stdlib work with lexical scope. If they don't, it's pretty hard to find that out since dynamic scope is so hard to reason about, giving another reason to make this change :)