Update to syn version 2 #299
Conversation
|
The failing tests come from some UI tests, the difference of one of them is here: Both of them have a similiar difference. Running the tests with the 1.79 and 1.80 compilers works fine, it is just nightly that seems to have changed the trait bound diagnostic message slightly. |
|
Yep, that happens from time to time and is easy for me to fix before the merge, no worries. The bigger issue is the code coverage failure caused by incompatibility between syn versions. |
binrw_derive/src/binrw/parser/mod.rs
Outdated
| let tokens = match &attr.meta { | ||
| syn::Meta::Path(_) => TokenStream::new(), | ||
| syn::Meta::List(ml) => ml.tokens.clone(), | ||
| syn::Meta::NameValue(nv) => nv.to_token_stream(), |
There was a problem hiding this comment.
This looks great! Really the only unaddressed question I have is regarding these new match arms. I believe that it had been the case that MetaAttrList would have failed a parse in parenthesized! before, but now that part of it has been uplifted into syn::Meta. I notice there is now a syn::Meta::require_list function. It seems to me like that should be used here instead?
There was a problem hiding this comment.
You are right! Since the parsing was moved up, we should error out when it is not a Meta::List. I added and rebased the necessary changes.
As there were quite substantial changes from syn version 1 to 2 some minor changes to the codebase are necessary: The `Attribute` type is more accurately typed now, that means instead of having simply a `Path` entity and some tokens after it, there is now a differentation between attributes with a `=`, in parentheses or without any other tokens but the path. This makes parsing slightly easier for us. Some test fixes were also required as these rely on the old tokens we got from an attribute. The `Spanned` trait was sealed in syn v2. We implemented it for some of the internal meta types. As we still have the span information readily available and the trait is otherwise required nowhere, simply remove the impl. Some syntax enums were marked as `non-exhaustive`. Handle these additional options in a sensible way where possible and throw compile errors in all other places.
|
I’ve squashed the relevant commits together with a couple of irrelevant tweaks and merged this. It will be in the next release. Thank you so much for doing this work! |
As there were quite substantial changes from syn version 1 to 2 some minor changes to the codebase are necessary:
The
Attributetype is more accurately typed now, that means instead of having simply aPathentity and some tokens after it, there is now a differentation between attributes with a=, in parentheses or without any other tokens but the path. This makes parsing slightly easier for us. Some test fixes were also required as these rely on the old tokens we got from an attribute.The
Spannedtrait was sealed in syn v2. We implemented it for some of the internal meta types. As we still have the span information readily available and the trait is otherwise required nowhere, simply remove the impl.Some syntax enums were marked as
non-exhaustive. Handle these additional options in a sensible way where possible and throw compile errors in all other places.