Grow the stack to allow parsing deeply nested expressions if necessary.

[hkratz]

parse_assoc_expr_with() appears to be the best place for ensure_sufficient_stack().

This could be performance-sensitive, so a perf run is required.

Fixes #79766, #93704

[rust-highfive]

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

[lqd]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit fcea6bf with merge b6f8940bd8fbcba4b12dc8019074b0b60e95bf89...

[bors]

☀️ Try build successful - checks-actions
Build commit: b6f8940bd8fbcba4b12dc8019074b0b60e95bf89 (b6f8940bd8fbcba4b12dc8019074b0b60e95bf89)

[rust-timer]

Queued b6f8940bd8fbcba4b12dc8019074b0b60e95bf89 with parent 926e784, future comparison URL.

[rust-timer]

Finished benchmarking commit (b6f8940bd8fbcba4b12dc8019074b0b60e95bf89): comparison url.

Summary: This benchmark run shows 33 relevant regressions 😿 to instruction counts.

• Average relevant regression: 0.7%
• Largest regression in instruction counts: 1.8% on incr-unchanged builds of deep-vector check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[petrochenkov]

I'm not even sure that it's something that needs to be fixed.
For every recursion case in the compiler it's possible to construct an artificial code example that will trigger stack overflow.
Then why grow the stack in this specific case but not in others? Just because someone to submitted an issue?

Stack overflow is not even an ICE or bug, just a regular runtime error caused by system resource exhaustion, like "disk full" or something.
We should be able to report such errors.

[petrochenkov]

parse_assoc_expr_with() appears to be the best place for ensure_sufficient_stack().

Could you elaborate why?

[hkratz]

parse_assoc_expr_with() appears to be the best place for ensure_sufficient_stack().

Could you elaborate why?

I thought that parse_assoc_expr_with() was called for all nested expressions but that is apparently not the case. I will have a closer look. Sorry about that.

[hkratz]

I'm not even sure that it's something that needs to be fixed. For every recursion case in the compiler it's possible to construct an artificial code example that will trigger stack overflow. Then why grow the stack in this specific case but not in others? Just because someone to submitted an issue?

My rationale was that of all syntactic constructs expressions are the most likely to be deeply nested and hitting the stack limit (e.g. when auto-generated). And since segmented stacks are already used in other parts of the compiler I thought that using them was common practice.

Stack overflow is not even an ICE or bug, just a regular runtime error caused by system resource exhaustion, like "disk full" or something. We should be able to report such errors.

With "disk full" the developer knows what to do: free some space or procure a larger disk. With stack overflows in large projects and generated code they likely do not even know where to look. Alternatively we could implement configurable recursion limits with conservative defaults for the parser just like clang. Then the developer gets at least told where the problem is.

[kostis]

Let me add my .02 here.

@petrochenkov is right in saying that

For every recursion case in the compiler it's possible to construct an artificial code example that will trigger stack overflow.

But there are two separate issues being reported:

1. Parens in (very!) long expression cause stack overflow #79766, resulting in a stack overflow message (and abort)
2. Segfault when compiling very deeply nested tuples #93704, which results in a segmentation fault.

IMO, something needs to be done about (at least) turning the segmentation fault into some other response (e.g. avoidance via automatic stack expansion or an abort with a "stack overflow" message ideally followed with a suggestion on how to overcome this).

[JohnCSimon]

@hkratz
Ping from triage: this seems to have gone idle due to inactivity, Please reopen if this is in error.
Thanks for your contribution.

@rustbot label: +S-inactive