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

Correctly consume tokens when parsing js_namespace #2510

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

afilini
Copy link
Contributor

@afilini afilini commented Mar 29, 2021

syn < 1.0.66 would consume the AnyIdent anyway while trying to parse the ExprArray.

Now due to a refactoring they did, they (correctly) leave the ParseBuffer untouched if the input is not a valid ExprArray, so we should avoid forking the input buffer and just use the same for both tries, otherwise the unwanted tokens left in there would make Punctuated break soon after.

Fixes #2508

`syn` < 1.0.66 would consume the `AnyIdent` anyway while trying to parse
the `ExprArray`.

Now due to a refactoring they did, they (correctly) leave the `ParseBuffer`
untouched if the input is not a valid `ExprArray`, so we should avoid forking
the input buffer and just use the same for both tries, otherwise the unwanted
tokens left in there would make `Punctuated` break soon after.

Fixes rustwasm#2508
@CryZe
Copy link
Contributor

CryZe commented Mar 29, 2021

You need to bump the nightly version here it seems (why is it pinned like this in the first place? seems odd):

toolchain: nightly-2021-01-23

Additionally wouldn't it make sense to also depend on the new syn if the old one had a bug (not sure I understand the problem with syn enough)

@alexcrichton
Copy link
Contributor

Thanks for the report and the PR! I've pushed up a few extra commits to fix CI and also update the syn version dependency. Will merge when green and then I'll do a version bump of wasm-bindgen itself.

@alexcrichton alexcrichton merged commit 5390654 into rustwasm:master Mar 29, 2021
@afilini afilini deleted the fix/syn-expr-array branch March 29, 2021 17:49
bors bot pushed a commit to bevyengine/bevy that referenced this pull request Mar 30, 2021
This is a temporary fix for the CI (and anyone building for wasm) break until `wgpu` can update

* `syn` released a version that fixed a bug in how they parsed attributes
* `wasm_bindgen` released a version that uses that fix
* but we're stuck with old `wasm_bindgen` as `wgpu` uses a fixed version: https://github.com/gfx-rs/wgpu-rs/blob/c5ee9cd98310aee66fb49bc98f4f65590304e4aa/Cargo.toml#L118

So, to fix this, either we update everyone to latest version of `wasm_bindgen` or we keep using old version of `syn`.

On Bevy side, it should be faster to fix the version of `syn` to one that works.

More details: rustwasm/wasm-bindgen#2510 & rustwasm/wasm-bindgen#2508
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.

Recent build failures from wasm-bindgen ecosystem
3 participants