Skip to content

Apply recursion limit without std - #2465

Merged
iffyio merged 3 commits into
apache:mainfrom
LucaCappelletti94:no-std-recursion-limit
Sep 3, 2026
Merged

Apply recursion limit without std#2465
iffyio merged 3 commits into
apache:mainfrom
LucaCappelletti94:no-std-recursion-limit

Conversation

@LucaCappelletti94

Copy link
Copy Markdown
Contributor

Parser::with_recursion_limit documented a hard parser depth limit, but builds without std used a stub RecursionCounter that always succeeded. Deeply nested input could overflow the stack instead of returning ParserError::RecursionLimitExceeded.

This makes the counter use alloc::rc::Rc and core::cell::Cell, which are available without std, and adds a --no default-features runtime test to the no-std CI path.

@LucaCappelletti94
LucaCappelletti94 marked this pull request as ready for review August 30, 2026 08:51
@LucaCappelletti94

Copy link
Copy Markdown
Contributor Author

@alamb since this version would be the first one fully support no-std, I would suggest we merge this one last no-std-related fix before the release

uses: ./.github/actions/setup-builder
with:
targets: 'thumbv6m-none-eabi'
- run: cargo test --release --no-default-features --test no_std_recursion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread src/parser/mod.rs

use super::ParserError;

/// Tracks remaining recursion depth. This value is decremented on

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove all these comments?

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @LucaCappelletti94

I took the liberty of merging up from main and touching up some comments

@iffyio iffyio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @LucaCappelletti94!

@iffyio
iffyio added this pull request to the merge queue Sep 3, 2026
Merged via the queue into apache:main with commit 66bc6b3 Sep 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants