MSRV: Make the code buildable on Rust 1.34 #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In Rust versions prior to 1.42, the proc_macro internal crate couldn't be used without this declaration, see https://github.com/rust-lang/rust/blob/master/RELEASES.md#cargo-4
Parameter-specific attributes are a feature of Rust 1.39, see https://github.com/rust-lang/rust/blob/master/RELEASES.md#language-7 for more details.
|
@laarmen Thank you for this PR, I have had a quick look at it and it seems reasonable. I will have a deeper look at your PR in the coming days. I have not decided on an MSRV policy for this crate yet, so I'll need to look into it as part of reviewing this. |
|
@laarmen I have finished looking at the MSRV policy and am ready to accept your PR after you apply the requested change. I will look into publishing a new release with the new MSRV policy as soon as possible. |
|
Hey! That was quick :)
I'll get on the modifications right away.
…On Wed, Sep 23, 2020 at 5:45 PM Emad Jacob Maroun ***@***.***> wrote:
@laarmen <https://github.com/laarmen> I have finished looking at the MSRV
policy and am ready to accept your PR after you apply the requested change.
I will look into publishing a new release with the new MSRV policy as soon
as possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEQ4NKQNRHMV2IGG5PLDOLSHIJXXANCNFSM4RTWXBAQ>
.
|
While this technically changes the semantics of the code, it shouldn't impact the overall behaviour of the method as a *normal* implementation of Iterator would be idempotent once the end reached. As such, if the first token is None, we can safely discard the second one. This change is done in order to avoid using a reference to `in_group` within the guard clause, as Rust 1.34 complains about it. It is probable that the more recent versions accept it from 1.39 on, see https://github.com/rust-lang/rust/blob/master/RELEASES.md#language-7
Being able to borrow a reference that has been bound-by-move in its guard clause has only been made possible in Rust 1.39, see https://github.com/rust-lang/rust/blob/master/RELEASES.md#language-7. Those changes are just a matter of specifying that we only want to match on references and not plain values.
laarmen
force-pushed
the
rust-1.34-compat
branch
from
99923f1
to
c05b00e
Compare
|
@laarmen Thanks for the changes, and thanks for the detailed commit messages. I'll merge this PR and look into publishing a new release. |
Emoun
approved these changes
Sep 24, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale
A crate I'm depending on is about to depend on
duplicate, and while the changes there seem really nice, I need to be able to build on 1.34.2 (which is the version of Rust shipped on the current Debian Stable).Description of the changes
The changes are relatively benign, with only one small semantic change, in
parse_utils::next_token, where in the inner group matching the function now unconditionally pops two values from the iterator instead of just one and "perhaps" another. As the iterator is locally created I don't think it leaks any side effect out of the function.You'll find some details about what wasn't 1.34-compatible in the commit messages.
Caveats
To be able to test those changes on 1.34, one need a patched version of macrotest.
Two tests fail on 1.34, but they fail because it's not possible to apply attributes to modules (module_disambiguation) nor statements (duplicate_inline) in 1.34, not because of the underlying logic of the macro.