Skip to content

feat(engine): make String.prototype.repeat respect loop iteration limit#5069

Merged
jedel1043 merged 2 commits intoboa-dev:mainfrom
alienx5499:feat/5060-runtime-limits-builtins
Mar 14, 2026
Merged

feat(engine): make String.prototype.repeat respect loop iteration limit#5069
jedel1043 merged 2 commits intoboa-dev:mainfrom
alienx5499:feat/5060-runtime-limits-builtins

Conversation

@alienx5499
Copy link
Contributor

This Pull Request fixes/closes #5060.

It changes the following:

  • Charge built-in iteration loops against the existing loop budget so they respect Context::set_loop_iteration_limit
  • Use IncrementLoopIteration::operation((), context) inside String.prototype.repeat so large repeat counts consume the VM loop budget and throw RuntimeLimitError::LoopIteration when the limit is exceeded
  • Add tests that:
    • Verify normal behavior is unchanged when no loop limit is set
    • With a small loop limit, both a plain JS loop and String.prototype.repeat throw the same RuntimeLimitError::LoopIteration
    • Include a regression test for a large repeat count to ensure the limit prevents unbounded work without panicking or hanging

Testing:

  • cargo test -p boa_engine (including the new runtime-limit tests for built-in iteration)

@alienx5499 alienx5499 requested a review from a team as a code owner March 14, 2026 18:57
@github-actions
Copy link

github-actions bot commented Mar 14, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 49,935 49,935 0
Ignored 2,207 2,207 0
Failed 821 821 0
Panics 0 0 0
Conformance 94.28% 94.28% 0.00%

Tested main commit: 50373f1c00129f6d2bb654241185f8e20e10a333
Tested PR commit: b7c53675db34e431fe74284ba1c7a70f0f219785
Compare commits: 50373f1...b7c5367

@alienx5499
Copy link
Contributor Author

Hi @jedel1043,

CI checks have passed for this PR. When you have some time, I’d appreciate a review.

This change focuses on the minimal entry point mentioned in the issue String.prototype.repeat to ensure builtins implemented with Rust loops correctly charge the VM loop iteration budget. If this approach looks good, I can follow up with additional PRs auditing other builtins that iterate internally.


std::iter::repeat_n(string.as_str(), n).for_each(|s| result.push(s));
// Charge each repetition against the VM loop-iteration limit.
let mut result = Vec::new();
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to carry over the with_capacity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve restored Vec::with_capacity(n) in the latest commit.

@jedel1043 jedel1043 added C-Builtins PRs and Issues related to builtins/intrinsics A-Internal Changes that don't modify execution behaviour and removed waiting-for-review labels Mar 14, 2026
@jedel1043 jedel1043 added this to the v1.0.0 milestone Mar 14, 2026
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks good!

@jedel1043 jedel1043 added the C-Tests Issues and PRs related to the tests. label Mar 14, 2026
@jedel1043 jedel1043 added this pull request to the merge queue Mar 14, 2026
Merged via the queue into boa-dev:main with commit 1dacd48 Mar 14, 2026
19 checks passed
@codecov
Copy link

codecov bot commented Mar 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.09%. Comparing base (6ddc2b4) to head (b7c5367).
⚠️ Report is 852 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5069       +/-   ##
===========================================
+ Coverage   47.24%   59.09%   +11.85%     
===========================================
  Files         476      563       +87     
  Lines       46892    62574    +15682     
===========================================
+ Hits        22154    36981    +14827     
- Misses      24738    25593      +855     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Internal Changes that don't modify execution behaviour C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime limits: ensure built‑in iteration helpers respect set_loop_iteration_limit

2 participants