Uh oh!
There was an error while loading. Please reload this page.
recursion_limit parsing handles overflows - #67272
Conversation
rust-highfive
commented
Dec 13, 2019
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. |
hellow554
commented
Dec 13, 2019
Can we add a test for this? Not sure how though. |
varkor
left a comment
There was a problem hiding this comment.
We should add a test for this. Other than that, and adding a warning, this change looks good.
Uh oh!
There was an error while loading. Please reload this page.
@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? |
fisherdarling
commented
Dec 16, 2019
@hellow554 This is some good advice, thank you! |
bors
commented
Dec 23, 2019
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
Dylan-DPC-zz
commented
Jan 16, 2020
@fisherdarling any updates on this? |
fisherdarling
commented
Jan 16, 2020
@Dylan-DPC Currently starting a new semester, I'm planning on getting an update out this weekend! |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
fisherdarling
commented
Jan 20, 2020
@Dylan-DPC Ready for any comments |
Uh oh!
There was an error while loading. Please reload this page.
varkor
left a comment
There was a problem hiding this comment.
Thanks for adding the tests, @fisherdarling! Just a few comments.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
joelpalmer
commented
Jan 27, 2020
Ping from Triage: any updates @fisherdarling? |
fisherdarling
commented
Jan 27, 2020
@joelpalmer Thanks for checking in. I'll get an update out tonight |
rust-highfive
commented
Feb 2, 2020
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 |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
varkor
commented
Feb 3, 2020
@fisherdarling: thanks! Could you squash your commits together? We avoid merge commits in the Rust repo. |
b0c61ea to
03922e3Comparerust-highfive
commented
Feb 4, 2020
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 |
rust-highfive
commented
Feb 4, 2020
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
left a comment
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.)
Uh oh!
There was an error while loading. Please reload this page.
JohnCSimon
commented
Feb 16, 2020
Ping from triage: @fisherdarling - can you please address the comments and changes from varkor? |
fisherdarling
commented
Feb 16, 2020
@varkor Finished up! I hope it looks good now. Thanks for all of the help! Apologies for my delay in getting this wrapped up. |
rust-highfive
commented
Feb 16, 2020
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 |
hellow554
commented
Feb 16, 2020
the |
hellow554
commented
Feb 17, 2020
After two months I think we're done. @varkor any last concerns? |
varkor
commented
Feb 17, 2020
@fisherdarling: thanks for bearing with all the comments! This looks good to go now! @bors r=varkor,hellow554 rollup |
bors
commented
Feb 17, 2020
📌 Commit c53693d has been approved by |
…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()`. closesrust-lang#67265
…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()`. closesrust-lang#67265
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
…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()`. closesrust-lang#67265
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
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