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

Support async/await and require Swift 5.5 #137

Closed
wants to merge 8 commits into from
Closed

Support async/await and require Swift 5.5 #137

wants to merge 8 commits into from

Conversation

MaxDesiatov
Copy link
Contributor

No description provided.

* wip

* Add sample

* Remove prototype

* Apply relative pointer patch for wasm

* Update example code to test without capturing context

* Remove prototype target

* WIP

* Update JSPromise interface

* Add cxxLanguageStandard option

* Strip unused headers

* Update toolchain version

* Exclude non-source files

* Fix example project build

* Update copy-header to download from remote source

* Adopt JSPromise changes

* Split Xcode support code

* Add API documents

* Adopt JSPromsie API change part2

* Add linguist-vendored

* Update README

* Revert JSObject changes
@MaxDesiatov MaxDesiatov changed the title Experimental async/await support (#112) Support async/await and require Swift 5.5 (#112) Sep 1, 2021
@kateinoigakukun
Copy link
Member

Things have been changed at all, so I need to make it re-work again 😅

@github-actions
Copy link

github-actions bot commented Sep 1, 2021

Time Change: +1,391.25ms (13%) ⚠️

Total Time: 10,326.75ms

Test name Duration Change
Serialization/Write JavaScript number directly 245.5ms +66.25ms (26%) 🚨
Serialization/Write JavaScript string directly 250.5ms +70.25ms (28%) 🚨
Serialization/Swift Int to JavaScript 3,237.25ms +421.75ms (13%) ⚠️
Serialization/Swift String to JavaScript 3,407.5ms +531.5ms (15%) ⚠️
Object heap/Increment and decrement RC 3,186ms +301.5ms (9%) 🔍

performance-action

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Sep 1, 2021

Fair enough, would it make sense to close this PR then?

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Sep 1, 2021

If you want to work on this, it makes sense to keep this open. If not, and no one will work on this, I'll do it in a different branch because most parts of this btanch are obsolete.

@MaxDesiatov
Copy link
Contributor Author

MaxDesiatov commented Sep 1, 2021

I would love to work on this, I only hoped it could be done quickly 😅

But since it needs a rewrite, I will probably be too slow with this. Let's keep it open for now to show our intent if anyone wants to know about our plans for this. Can close this one when a separate branch/PR for a rewrite is needed.

@MaxDesiatov MaxDesiatov changed the title Support async/await and require Swift 5.5 (#112) Support async/await and require Swift 5.5 Sep 2, 2021
@MaxDesiatov
Copy link
Contributor Author

@kateinoigakukun have you seen error messages in CI runs here? They were caused by swiftasync CC, which is apparently not supported by Clang when targeting Wasm. Is this no longer an issue in your replacement PR?

@kateinoigakukun
Copy link
Member

@MaxDesiatov

At first, my new PR doesn't bring headers from toolchain, so there is no issue around it.

Then, think about the issue here.
I think that error will be gone after updating the Config.h to be the latest version.
it tries to use swiftasynccall in JSKit, but not supported on wasm https://github.com/swiftwasm/JavaScriptKit/pull/137/files#diff-e748bb05e91769e7ca05e9fd4ea4b4ff6b53c183d38d7b455fc4e52eb5860572R181-R185
On the toolchain side, the attribute is not used due to __has_feature(swiftasynccc) condition.
https://github.com/swiftwasm/swift/blob/b1d209396b535d4af0613ff1aa5deebd94e329a3/include/swift/Runtime/Config.h#L207

@MaxDesiatov MaxDesiatov closed this Oct 4, 2021
@MaxDesiatov MaxDesiatov deleted the next branch March 31, 2022 09:43
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.

2 participants