recursion_limit parsing handles overflows #67272
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
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @varkor (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
fisherdarling
changed the title
|
Can we add a test for this? Not sure how though. |
varkor
suggested changes
Dec 13, 2019
|
@fisherdarling an introduction to the various test directories and functions can be found at https://rust-lang.github.io/rustc-guide/tests/intro.html I would add a folder in @varkor would you do anything different here? |
|
@hellow554 This is some good advice, thank you! |
|
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
Dylan-DPC-zz
added
S-waiting-on-author
|
@fisherdarling any updates on this? |
|
@Dylan-DPC Currently starting a new semester, I'm planning on getting an update out this weekend! |
fisherdarling
commented
Jan 20, 2020
|
@Dylan-DPC Ready for any comments |
hellow554
reviewed
Jan 20, 2020
varkor
suggested changes
Jan 20, 2020
There was a problem hiding this comment.
Thanks for adding the tests, @fisherdarling! Just a few comments.
|
Ping from Triage: any updates @fisherdarling? |
|
@joelpalmer Thanks for checking in. I'll get an update out tonight |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
fisherdarling
commented
Feb 3, 2020
hellow554
reviewed
Feb 3, 2020
varkor
reviewed
Feb 3, 2020
varkor
reviewed
Feb 3, 2020
varkor
reviewed
Feb 3, 2020
|
@fisherdarling: thanks! Could you squash your commits together? We avoid merge commits in the Rust repo. |
fisherdarling
force-pushed
the
master
branch
2 times, most recently
from
b0c61ea
to
03922e3
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
varkor
reviewed
Feb 8, 2020
There was a problem hiding this comment.
Sorry for taking a while to review this again. I spotted one last thing — but after this, everything looks good! (Something seems to have happened to the committer information when you rebased, as you're listed twice, but only one is linked with your GitHub account, which you might want to look at, but it's not a big problem.)
|
Ping from triage: @fisherdarling - can you please address the comments and changes from varkor? |
fisherdarling
force-pushed
the
master
branch
from
3102780
to
d27b4af
Compare
|
@varkor Finished up! I hope it looks good now. Thanks for all of the help! Apologies for my delay in getting this wrapped up. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
the |
fisherdarling
force-pushed
the
master
branch
from
f9e1531
to
c53693d
Compare
|
After two months I think we're done. @varkor any last concerns? |
|
@fisherdarling: thanks for bearing with all the comments! This looks good to go now! @bors r=varkor,hellow554 rollup |
|
📌 Commit c53693d has been approved by |
bors
added
S-waiting-on-bors
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Feb 17, 2020
…ow554 recursion_limit parsing handles overflows This PR adds overflow handling to `#![recursion_limit]` attribute parsing. If parsing the given value results in an `IntErrorKind::Overflow`, then the recursion_limit is set to `usize::max_value()`. closes rust-lang#67265
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Feb 18, 2020
…ow554 recursion_limit parsing handles overflows This PR adds overflow handling to `#![recursion_limit]` attribute parsing. If parsing the given value results in an `IntErrorKind::Overflow`, then the recursion_limit is set to `usize::max_value()`. closes rust-lang#67265
bors
added a commit
that referenced
this pull request
Feb 18, 2020
Rollup of 8 pull requests Successful merges: - #67272 (recursion_limit parsing handles overflows) - #68597 (Simplify `Skip::nth` and `Skip::last` implementations) - #68767 (macOS: avoid calling pthread_self() twice) - #69175 (Do not ICE when encountering `yield` inside `async` block) - #69223 (Ignore GDB versions with broken str printing.) - #69244 (configure: set LLVM flags with a value) - #69249 (Stabilize {f32, f64}::{LOG2_10, LOG10_2}) - #69252 (Clean out unused directories for extra disk space) Failed merges: r? @ghost
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Feb 18, 2020
…ow554 recursion_limit parsing handles overflows This PR adds overflow handling to `#![recursion_limit]` attribute parsing. If parsing the given value results in an `IntErrorKind::Overflow`, then the recursion_limit is set to `usize::max_value()`. closes rust-lang#67265
bors
added a commit
that referenced
this pull request
Feb 18, 2020
Rollup of 8 pull requests Successful merges: - #67272 (recursion_limit parsing handles overflows) - #68597 (Simplify `Skip::nth` and `Skip::last` implementations) - #68767 (macOS: avoid calling pthread_self() twice) - #69175 (Do not ICE when encountering `yield` inside `async` block) - #69223 (Ignore GDB versions with broken str printing.) - #69244 (configure: set LLVM flags with a value) - #69249 (Stabilize {f32, f64}::{LOG2_10, LOG10_2}) - #69252 (Clean out unused directories for extra disk space) Failed merges: r? @ghost
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.
This PR adds overflow handling to
#![recursion_limit]attribute parsing. If parsing the given value results in anIntErrorKind::Overflow, then the recursion_limit is set tousize::max_value().closes #67265