From 76b7b8dae21aa4bd14c717fafd223b56c1e77afd Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:39:15 -0700 Subject: [PATCH 1/2] fix(lang): track single-quote literals in structure-marker strip (#866) langstripstructuremarkers scans each source line to find where a trailing comment begins, so that a comment marker inside a string literal is not mistaken for a real comment. It tracked two of UserTalk's three literal delimiters -- ASCII " and the Mac curly-quote pair (0xD2/0xD3) -- but not chsinglequote (0x27), which delimits character and string4 constants. On a line such as if s contains 'C' { (where C is chcomment, 0xC7) the 0xC7 inside the literal was read as the start of a trailing comment. commentstart landed mid-literal, so the trailing structural-marker strip was skipped for the real content and a { survived in the stored node text. The outline export (oplangtextvisit) then re-emits a { from the outline LEVEL transition, so the line gained one extra brace per install: install 1: if s contains 'C' { { install 2: if s contains 'C' { { { The growth is cumulative and unbounded, and the script stops compiling from the first install onward. This is the same family as #621, which taught the strip about inline one-line blocks but not the scanner about single-quote literals. Both scanners in the function had the gap: the comment-start scan and the brace-balance counter that decides whether a trailing } is load-bearing. A brace inside a character constant is not structural, so both now track all three delimiters. The two-way flqcurly/qcurly boolean is replaced by an explicit closing-delimiter byte, which extends to three delimiters without further branching. Effect on the shipped corpus: a sweep of all 2,996 scripts in Virgin.root (round-trip each, compile-gated with an installer- unrepairable negative control) had exactly one script that reinstall left non-compiling -- suites.commercial.parseAete, an instance of this exact shape. It is now stable across repeated reinstalls. Tests: three cases added to script_install_roundtrip.yaml (read-back byte-equality, reinstall idempotence, and end-to-end execution). Verified red before the fix and green after; the 8 pre-existing cases in that file pass throughout. Unit-test result sets are identical to baseline (the test_callback_infrastructure segfault reproduces unchanged on unmodified develop and is unrelated). Note for test authors: the runner writes script files as UTF-8, so a literal 0xC7 byte in YAML arrives as two bytes and breaks a single-quoted character constant. These tests use the \xc7 escape, which is ASCII in the file and decodes to the single byte at runtime. --- Common/source/langscan.c | 50 +++++++++++-------- .../test_cases/script_install_roundtrip.yaml | 39 +++++++++++++++ 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/Common/source/langscan.c b/Common/source/langscan.c index 7bdbd03e2..2e0670738 100644 --- a/Common/source/langscan.c +++ b/Common/source/langscan.c @@ -1380,34 +1380,39 @@ boolean langstripstructuremarkers (Handle hin, Handle *hout) { /*scan forward from the indent boundary to find the start of any trailing comment (« or //) outside a string literal. UserTalk - strings use ASCII " (chdoublequote, 0x22) OR the Mac smart-quote - pair chopencurlyquote (0xD2) / chclosecurlyquote (0xD3); the - scanner accepts both forms (langscan.c parsepopstringconst), so - we track both here. Without that, a string like "a // b" with - "" delimiters works, but "a // b" with «...» smart quotes would - get its // misread as a comment start and the rest of the line - truncated. flqcurly distinguishes the closing delimiter to use.*/ + has three literal delimiters, and all three must be tracked here + or a comment marker inside one gets misread as a real comment: + ASCII " (chdoublequote, 0x22); the Mac smart-quote pair + chopencurlyquote (0xD2) / chclosecurlyquote (0xD3); and + chsinglequote (0x27) for character and string4 constants + (parsepopstringconst / the single-quote branch below both accept + these). Without that, a string like "a // b" with "" delimiters + works, but «a // b» or 'Ç' would get its comment byte misread as + a comment start, truncating the rest of the line. Losing the + trailing structural-marker strip that way leaves a `{` in the + stored node text that the outline export re-emits from the level + transition, so each reinstall adds another brace (#866). + chclose is the delimiter that ends the literal we are inside.*/ long i = linestart + indent; - boolean flqcurly = false; + byte chclose = 0; while (i < lineend) { byte ch = buf [i]; if (flinstring) { /*\ escapes the next byte (e.g. \" inside ASCII strings). - Same convention applies inside curly-quote strings; the - scanner doesn't distinguish.*/ + Same convention applies inside curly-quote and + single-quote strings; the scanner doesn't distinguish.*/ if (ch == '\\' && i + 1 < lineend) { i += 2; continue; } - if ((!flqcurly && ch == '"') - || (flqcurly && ch == (byte) chclosecurlyquote)) + if (ch == chclose) flinstring = false; ++i; continue; } - if (ch == '"' || ch == chopencurlyquote) { + if (ch == '"' || ch == chopencurlyquote || ch == (byte) chsinglequote) { flinstring = true; - flqcurly = (ch == chopencurlyquote); + chclose = (ch == chopencurlyquote) ? (byte) chclosecurlyquote : ch; ++i; continue; } @@ -1453,13 +1458,17 @@ boolean langstripstructuremarkers (Handle hin, Handle *hout) { surplus closing braces. If not, the brace we are about to strip is needed to close an inline one-line block; stop. Count {/} in the kept content, respecting - string-literal state for both " and « forms (escape - with backslash applies to both).*/ + string-literal state for the ", « and ' forms (escape + with backslash applies to all three). Single quotes + must be tracked here for the same reason as in the + comment-start scan above: a brace inside a character + or string4 constant is not structural (#866). + k_close is the delimiter ending the current literal.*/ long try_from = stripfrom - 1; long open_ct = 0; long close_ct = 0; boolean in_str = false; - boolean qcurly = false; + byte k_close = 0; boolean esc = false; long k; for (k = linestart + indent; k < try_from; ++k) { @@ -1467,14 +1476,13 @@ boolean langstripstructuremarkers (Handle hin, Handle *hout) { if (esc) { esc = false; continue; } if (in_str) { if (kc == '\\') esc = true; - else if ((!qcurly && kc == '"') - || (qcurly && kc == (byte) chclosecurlyquote)) + else if (kc == k_close) in_str = false; continue; } - if (kc == '"' || kc == chopencurlyquote) { + if (kc == '"' || kc == chopencurlyquote || kc == (byte) chsinglequote) { in_str = true; - qcurly = (kc == chopencurlyquote); + k_close = (kc == chopencurlyquote) ? (byte) chclosecurlyquote : kc; continue; } if (kc == '{') ++open_ct; diff --git a/tests/integration/test_cases/script_install_roundtrip.yaml b/tests/integration/test_cases/script_install_roundtrip.yaml index ca0640ae2..70f0cac09 100644 --- a/tests/integration/test_cases/script_install_roundtrip.yaml +++ b/tests/integration/test_cases/script_install_roundtrip.yaml @@ -111,6 +111,45 @@ tests: expected_success: true expected_result: "true" + - name: "round-trip: comment marker inside a single-quoted literal is not a comment (#866)" + description: "A single-quoted character/string4 constant holding the comment-marker byte (0xC7) must round-trip unchanged. The strip pass tracked \" and the curly-quote pair but not ', so the 0xC7 inside 'C' was mis-read as the start of a trailing comment. That set commentstart mid-literal, skipped the trailing structural-marker strip, and left a { in the stored node text that the export pass re-emitted from the level transition -- adding one extra { per install, cumulatively, until the script no longer compiled." + script: | + local (src = "on f () {\r\tlocal (s = \"x\");\r\tif s contains '\xc7' {\r\t\ts = \"y\"};\r\treturn (s)}"); + new (tabletype, @system.temp.rt); + script.newScriptObject (src, @system.temp.rt.f); + local (back = string (system.temp.rt.f)); + delete (@system.temp.rt); + return (back == src) + expected_success: true + expected_result: "true" + + - name: "round-trip: single-quoted comment marker stays idempotent across reinstall (#866)" + description: "Reinstalling a script's own unmodified source must be idempotent. With the single-quote gap, each install appended another { to the same line, so the second read-back differed from the first and the script stopped compiling." + script: | + local (src = "on f () {\r\tlocal (s = \"x\");\r\tif s contains '\xc7' {\r\t\ts = \"y\"};\r\treturn (s)}"); + new (tabletype, @system.temp.rt); + script.newScriptObject (src, @system.temp.rt.f); + local (first = string (system.temp.rt.f)); + script.newScriptObject (first, @system.temp.rt.f); + local (second = string (system.temp.rt.f)); + delete (@system.temp.rt); + return (first == second) + expected_success: true + expected_result: "true" + + - name: "round-trip: single-quoted comment marker still compiles and runs (#866)" + description: "End-to-end behavioral check: after install the script must actually execute, not merely read back. suites.commercial.parseAete in Virgin.root is the real-world instance of this shape." + script: | + local (src = "on f (s) {\r\tif s contains '\xc7' {\r\t\treturn (1)};\r\treturn (0)}"); + new (tabletype, @system.temp.rt); + script.newScriptObject (src, @system.temp.rt.f); + local (hit = system.temp.rt.f ("a\xc7b")); + local (miss = system.temp.rt.f ("ab")); + delete (@system.temp.rt); + return (hit == 1 and miss == 0) + expected_success: true + expected_result: "true" + - name: "round-trip: inline one-line nested block at outermost depth (#621)" description: "Same shape as the previous test but the inline block sits at the outermost body level (no enclosing if/while). Confirms the brace-balance check generalizes across nesting depths and isn't depth-1-specific." script: | From 2b11766c4758aa1fdbb0674fc2295030e7ae3a29 Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:51:45 -0700 Subject: [PATCH 2/2] test(lang): round-trip coverage for the delimiter refactor (#866) Follow-up to 76b7b8dae, from the bar-raiser verdict. Test-only; no source changes. - Fix a stale cross-reference. The three #866 cases were inserted between the two #621 cases, so "Same shape as the previous test" in the outermost-depth case pointed at the wrong neighbour. It now names the depth-1 inline-block case explicitly, which survives reordering. - Add a brace-balance-counter case. The fix touched two scanners; the three #866 cases only name the comment-start scan. This one puts a brace inside a character constant on a line that also ends with an inline one-line block, so it runs through the counter that decides whether a trailing } is load-bearing. - Add a curly-quote (0xD2/0xD3) string-literal case. That delimiter was tracked by both scanners but had no test, and the refactor rewrote how the closing delimiter is chosen. Status of the two new cases, measured rather than assumed: both PASS on a pre-fix binary built from 76b7b8dae~1, so they are regression guards for the refactor, not reproductions of the bug. Only the three original #866 cases are red pre-fix (verified through the runner against that binary: 10 passed / 3 failed). All 13 pass at this commit. The new cases are ASCII in the yaml and use \xd2 \xd3 \xc7 escapes; the runner writes script files as UTF-8, so a literal high byte would arrive as two bytes and break a single-quoted constant. --- .../test_cases/script_install_roundtrip.yaml | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_cases/script_install_roundtrip.yaml b/tests/integration/test_cases/script_install_roundtrip.yaml index 70f0cac09..31d5626cd 100644 --- a/tests/integration/test_cases/script_install_roundtrip.yaml +++ b/tests/integration/test_cases/script_install_roundtrip.yaml @@ -150,8 +150,33 @@ tests: expected_success: true expected_result: "true" + - name: "round-trip: brace inside a single-quoted constant is not counted as structure (#866)" + description: "Exercises the brace-balance counter rather than the comment-start scan. That counter decides whether a trailing } is load-bearing (the #621 inline-block check); it tracked the same two delimiters and so counted a { or } inside a character constant as structural. Here the constant holds a brace and the line ends with an inline one-line block, so a miscount changes the strip decision and unbalances the stored node text. Both scanners now track ' as a delimiter." + script: | + local (src = "on f (c) {\r\tif c == '{' {\r\t\ttry {local (x = 1)}};\r\treturn (1)}"); + new (tabletype, @system.temp.rt); + script.newScriptObject (src, @system.temp.rt.f); + local (back = string (system.temp.rt.f)); + local (result = system.temp.rt.f ("{")); + delete (@system.temp.rt); + return (back == src and result == 1) + expected_success: true + expected_result: "true" + + - name: "round-trip: comment marker inside a curly-quote string literal (0xD2/0xD3)" + description: "Coverage gap predating #866: the curly-quote string form was tracked by both scanners but never exercised by a test, and the delimiter refactor touches that path. A comment marker inside a Mac smart-quote string must not be read as a trailing comment. Uses the \\xd2 and \\xd3 escapes so the bytes survive the runner writing the script file as UTF-8." + script: | + local (src = "on f () {\r\tlocal (s = \xd2a \xc7 b\xd3);\r\treturn (s)}"); + new (tabletype, @system.temp.rt); + script.newScriptObject (src, @system.temp.rt.f); + local (back = string (system.temp.rt.f)); + delete (@system.temp.rt); + return (back == src) + expected_success: true + expected_result: "true" + - name: "round-trip: inline one-line nested block at outermost depth (#621)" - description: "Same shape as the previous test but the inline block sits at the outermost body level (no enclosing if/while). Confirms the brace-balance check generalizes across nesting depths and isn't depth-1-specific." + description: "Same shape as the depth-1 inline-block case above but the inline block sits at the outermost body level (no enclosing if/while). Confirms the brace-balance check generalizes across nesting depths and isn't depth-1-specific." script: | local (src = "on f () {\r\ttry {local (x = 7)};\r\treturn (1)}"); new (tabletype, @system.temp.rt);