Skip to content

Accept a comment directly after a number - #136

Merged
sheredom merged 1 commit into
sheredom:mainfrom
youdie006:comment-after-number
Oct 6, 2026
Merged

sheredom merged 1 commit into
sheredom:mainfrom
youdie006:comment-after-number

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

With json_parse_flags_allow_c_style_comments (and so allow_json5), a comment is accepted after a string, a literal or a space, but not directly after a number:

input main this PR
[1/*x*/,2] error 5 (invalid_number_format) at offset 2 [1,2]
{a:1// x\n} error 5 parses
[1 /*x*/,2], ["a"/*x*/,2], [true/*x*/] parse parse

The number terminator switch in json_get_number_size has a case for = under allow_equals_in_object but none for /. This adds the same case for / under allow_c_style_comments; without the flag it is still invalid_number_format. npm json5 parses the test payload [-1.5e3/* a */,2// b\n] to [-1500,2].

after_number and after_number_without_flag follow the other allow_c_style_comments tests in test/test_shared.h; the first fails on main. The suite passes in gcc Debug and Release builds and in a Debug build with ASan and UBSan, and inputs ending right after the / (1/, [1/, [1/*) read past nothing. I did not run clang, tcc or MSVC.

An unterminated /* at the end of the input is accepted (1 /* on main too), so 1/* now parses; json_skip_c_style_comments returns 1 for it although its comment calls it a failure. I can send that separately.

Written with AI assistance (Claude); I have reviewed the change.

With json_parse_flags_allow_c_style_comments, a comment is accepted
after a string, literal or a space, but json_get_number_size only ended
a number at whitespace, '}', ',', ']' or '=', so [1/* a */,2] failed
with invalid_number_format. Let '/' end a number when comments are on.
@sheredom
sheredom merged commit 95e5cdb into sheredom:main Oct 6, 2026
24 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.

2 participants