Uh oh!
There was an error while loading. Please reload this page.
Ensure stack when type checking and building MIR for large if expressions - #74708
Merged
Conversation
rust-highfive
commented
Jul 24, 2020
Contributor
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (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. |
davidtwco
commented
Jul 24, 2020
Member
@bors r+ |
bors
commented
Jul 24, 2020
Collaborator
📌 Commit f64e58c has been approved by |
Dylan-DPC-zz pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Jul 24, 2020
Ensure stack when type checking and building MIR for large if expressions Fixesrust-lang#74564
mati865
commented
Jul 24, 2020
Member
Is is safe for rollups, perf wise? |
davidtwco
commented
Jul 24, 2020
Member
I'm not sure, I'd assume so? |
mati865
commented
Jul 24, 2020
Member
Docs are not that convincing 😅:
|
JohnTitor
commented
Jul 24, 2020
Member
Failed in rollup: #74719 (comment) |
kanru
commented
Jul 25, 2020
ContributorAuthor
Hmm, the test passed locally on my machine. I wonder what makes the rollup to fail 🤔 |
JohnTitor
commented
Jul 25, 2020
Member
@kanru I can reproduce the failure on my local, you should rebase. |
kanru
commented
Jul 25, 2020
ContributorAuthor
I can reproduce with stage 2. Investigating... |
davidtwco
approved these changes
Jul 26, 2020
davidtwco
commented
Jul 26, 2020
Member
@bors r+ |
bors
commented
Jul 26, 2020
Collaborator
📌 Commit 0e2ec8c has been approved by |
davidtwco
commented
Jul 26, 2020
Member
@bors rollup=never (in case there is a perf impact) |
bors
commented
Jul 26, 2020
Collaborator
bors
commented
Jul 27, 2020
Collaborator
☀️ Test successful - checks-actions, checks-azure |
cuviper added a commit
to cuviper/rust
that referenced
this pull request
Aug 14, 2025
In Fedora, when we built rustc with PGO on ppc64le, we started failing the test `issue-74564-if-expr-stack-overflow.rs`. This could also be reproduced on other arches by setting a smaller `RUST_MIN_STACK`, so it's probably just unlucky that ppc64le PGO created a large stack frame somewhere in this recursion path. Adding an `ensure_sufficient_stack` solves the stack overflow. Historically, that test and its fix were added in rust-lang#74708, which was also an `ensure_sufficient_stack` in this area of code at the time. However, the refactor in rust-lang#92573 basically left that to the general `MutVisitor`, and then rust-lang#142240 removed even that ensure call. It may be luck that our tier-1 tested targets did not regress the original issue across those refactors.
bors added a commit
that referenced
this pull request
Aug 22, 2025
rustc_expand: ensure stack in `InvocationCollector::visit_expr` In Fedora, when we built rustc with PGO on ppc64le, we started failing the test `issue-74564-if-expr-stack-overflow.rs`. This could also be reproduced on other arches by setting a smaller `RUST_MIN_STACK`, so it's probably just unlucky that ppc64le PGO created a large stack frame somewhere in this recursion path. Adding an `ensure_sufficient_stack` solves the stack overflow. Historically, that test and its fix were added in #74708, which was also an `ensure_sufficient_stack` in this area of code at the time. However, the refactor in #92573 basically left that to the general `MutVisitor`, and then #142240 removed even that ensure call. It may be luck that our tier-1 tested targets did not regress the original issue across those refactors.
github-actionsBot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Aug 24, 2025
rustc_expand: ensure stack in `InvocationCollector::visit_expr` In Fedora, when we built rustc with PGO on ppc64le, we started failing the test `issue-74564-if-expr-stack-overflow.rs`. This could also be reproduced on other arches by setting a smaller `RUST_MIN_STACK`, so it's probably just unlucky that ppc64le PGO created a large stack frame somewhere in this recursion path. Adding an `ensure_sufficient_stack` solves the stack overflow. Historically, that test and its fix were added in rust-lang/rust#74708, which was also an `ensure_sufficient_stack` in this area of code at the time. However, the refactor in rust-lang/rust#92573 basically left that to the general `MutVisitor`, and then rust-lang/rust#142240 removed even that ensure call. It may be luck that our tier-1 tested targets did not regress the original issue across those refactors.
github-actionsBot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Aug 25, 2025
rustc_expand: ensure stack in `InvocationCollector::visit_expr` In Fedora, when we built rustc with PGO on ppc64le, we started failing the test `issue-74564-if-expr-stack-overflow.rs`. This could also be reproduced on other arches by setting a smaller `RUST_MIN_STACK`, so it's probably just unlucky that ppc64le PGO created a large stack frame somewhere in this recursion path. Adding an `ensure_sufficient_stack` solves the stack overflow. Historically, that test and its fix were added in rust-lang/rust#74708, which was also an `ensure_sufficient_stack` in this area of code at the time. However, the refactor in rust-lang/rust#92573 basically left that to the general `MutVisitor`, and then rust-lang/rust#142240 removed even that ensure call. It may be luck that our tier-1 tested targets did not regress the original issue across those refactors.
Kobzol pushed a commit
to Kobzol/rustc_codegen_gcc
that referenced
this pull request
Dec 21, 2025
rustc_expand: ensure stack in `InvocationCollector::visit_expr` In Fedora, when we built rustc with PGO on ppc64le, we started failing the test `issue-74564-if-expr-stack-overflow.rs`. This could also be reproduced on other arches by setting a smaller `RUST_MIN_STACK`, so it's probably just unlucky that ppc64le PGO created a large stack frame somewhere in this recursion path. Adding an `ensure_sufficient_stack` solves the stack overflow. Historically, that test and its fix were added in rust-lang/rust#74708, which was also an `ensure_sufficient_stack` in this area of code at the time. However, the refactor in rust-lang/rust#92573 basically left that to the general `MutVisitor`, and then rust-lang/rust#142240 removed even that ensure call. It may be luck that our tier-1 tested targets did not regress the original issue across those refactors.
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Fixes#74564