Skip to content

src: use constexpr table for GetOctalCode#64471

Open
ansh1406 wants to merge 1 commit into
nodejs:mainfrom
ansh1406:src-constexpr-octal-table
Open

src: use constexpr table for GetOctalCode#64471
ansh1406 wants to merge 1 commit into
nodejs:mainfrom
ansh1406:src-constexpr-octal-table

Conversation

@ansh1406

Copy link
Copy Markdown

Replaced the runtime std::vector<std::string> cache used by GetOctalCode() with a compile-time constexpr lookup table of std::string_view.

Changes

  1. GetOctalCode() now returns std::string_view instead of const std::string&.

  2. There are two call sites that use GetOctalCode():

    • src/node_snapshotable.cc: The returned value is streamed directly to an std::ostream, so no changes were required.
    • tools/js2c.cc: Updated the implementation to use std::string_view::data() instead of std::string::c_str().

Verification

  • Verified that the generated output is identical for all 256 byte values.
  • Full build completed successfully on Debian 13.4 (x86_64, GCC, make).

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jul 13, 2026
Replace the runtime std::vector<std::string> cache in
GetOctalCode() with a compile-time constexpr lookup table of
std::string_view

Signed-off-by: Ansh Swaroop <anshswaroop14@gmail.com>
@ansh1406 ansh1406 force-pushed the src-constexpr-octal-table branch from eb5cd72 to 9e25225 Compare July 13, 2026 13:33
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (b4c7be3) to head (9e25225).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64471      +/-   ##
==========================================
+ Coverage   90.23%   90.24%   +0.01%     
==========================================
  Files         741      741              
  Lines      241604   241593      -11     
  Branches    45520    45520              
==========================================
+ Hits       218010   218027      +17     
+ Misses      15120    15113       -7     
+ Partials     8474     8453      -21     
Files with missing lines Coverage Δ
src/embedded_data.cc 100.00% <100.00%> (+10.52%) ⬆️

... and 35 files with indirect coverage changes

🚀 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.

@ansh1406

Copy link
Copy Markdown
Author

lint-js-and-md check got timed out for some reason. My changes are out of it's scope. Can a maintainer please re run it if it's necessary.

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants