Skip to content

Normalize string literal representation across the compiler - #8606

Open
cknitt wants to merge 4 commits into
codex/string-literal-regression-coveragefrom
codex/string-literal-representation
Open

Normalize string literal representation across the compiler#8606
cknitt wants to merge 4 commits into
codex/string-literal-regression-coveragefrom
codex/string-literal-representation

Conversation

@cknitt

@cknitt cknitt commented Sep 1, 2026

Copy link
Copy Markdown
Member

This is the second PR in the two-PR String Theory stack, built on #8605. It replaces the compiler’s historical delimiter protocol with explicit representations for semantic strings, templates, JSON literals, and raw JavaScript.

Consistent representations

Each layer now distinguishes runtime string values from source text that must be preserved:

Concept Representation
Ordinary string Source and semantic value in the Parsetree; semantic value only after type checking
Ordinary template Explicit template node with validated segments containing source and semantic forms
Tagged template Explicit tagged-template node with raw source segments
JSON literal Dedicated validated JavaScript-source representation
Raw JavaScript Dedicated representation carrying its raw-code kind

This structure continues through the typed tree, Lambda, and JavaScript IR. The frozen AST v0 bridge converts to and from its legacy encoding at that single compatibility boundary.

A shared String_literal module now owns escape decoding, canonical encoding, UTF-16 operations, surrogate handling, and template line-ending semantics.

This removes:

  • the DNone, DStarJ, DBackQuotes, and DNoQuotes delimiter protocol
  • the "js", "*j", and "bq" marker strings
  • hidden ++ trees and attributes for interpolated templates
  • synthetic calls used to represent tagged templates
  • the legacy Ast_utf8_string and Ast_utf8_string_interp implementations

Output behavior

Template literals now remain explicit throughout compilation and carry over into the generated JavaScript:

  • Ordinary backquoted literals are emitted as backquoted JavaScript literals, preserving their source spelling.
  • Interpolated templates are emitted as native JavaScript template literals instead of being lowered to hidden ++ applications.
  • Tagged templates are emitted as genuine JavaScript tagged templates, preserving their raw segments and JavaScript cooked/raw behavior.
  • Interpolated values retain their original left-to-right, once-only evaluation behavior.
  • Ordinary strings are emitted from their semantic value through one canonical escaping path.
  • JSON literals and raw JavaScript remain source payloads and are emitted as JavaScript syntax rather than quoted strings.

Behavior fixes

The normalized representation also resolves several correctness issues:

  • Equivalent patterns such as "a" and "\x61" now compare by runtime value, preserve source order, and produce the expected redundant-pattern warning.
  • Constant folding uses JavaScript UTF-16 semantics for string length and indexing.
  • Valid UTF-16 surrogate pairs are accepted, while lone surrogates, malformed UTF-8, and invalid escapes are rejected.
  • Template CR and CRLF source line endings are normalized according to JavaScript semantics.
  • U+2028 and U+2029 line continuations are decoded correctly.
  • Interpolation-free templates remain nonexpansive and preserve value generalization.
  • Tagged templates are rejected in patterns because patterns cannot invoke their tag.
  • json interpolation and unsupported uses of JSON literals are rejected.
  • Ordinary template escapes are validated while tagged-template raw segments may still contain invalid escapes.
  • External PPX round trips preserve template spelling and escape-sensitive raw payloads.
  • GenType correctly escapes semantic @as values and Unicode line separators in generated paths.

Testing

Coverage spans parsing and printing, the AST v0 bridge, type checking, Lambda and JavaScript IR, constant folding, generated JavaScript, GenType, analysis, and end-to-end behavior.

Verified with:

  • make test
  • make test-syntax
  • make test-syntax-roundtrip
  • make test-gentype
  • make test-analysis

Closes #8602.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T19:59:53.109198Z 513d122 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cknitt
cknitt force-pushed the codex/string-literal-representation branch from a6f1445 to c292e07 Compare September 1, 2026 19:04
@cknitt cknitt changed the title codex/string literal representation Normalize string literal representation across the compiler Sep 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6f1445259

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread compiler/ml/parsetree.ml
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.20568% with 263 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.67%. Comparing base (97fb765) to head (cdf45ee).

Files with missing lines Patch % Lines
tests/ounit_tests/ounit_string_literal_tests.ml 88.27% 38 Missing ⚠️
compiler/ml/ast_mapper_from0.ml 77.21% 18 Missing ⚠️
compiler/core/js_exp_make.ml 75.38% 16 Missing ⚠️
tests/ounit_tests/ounit_ast_mapper0_tests.ml 89.93% 15 Missing ⚠️
analysis/src/dump_ast.ml 7.69% 12 Missing ⚠️
compiler/ml/printtyped.ml 0.00% 12 Missing ⚠️
compiler/ml/printast.ml 0.00% 11 Missing ⚠️
analysis/reanalyze/src/annotation.ml 57.89% 8 Missing ⚠️
compiler/core/lam_compile_primitive.ml 42.85% 8 Missing ⚠️
compiler/syntax/src/res_scanner.ml 68.00% 8 Missing ⚠️
... and 39 more
Additional details and impacted files
@@                             Coverage Diff                              @@
##           codex/string-literal-regression-coverage    #8606      +/-   ##
============================================================================
+ Coverage                                     76.48%   76.67%   +0.18%     
============================================================================
  Files                                           478      478              
  Lines                                         63172    63864     +692     
============================================================================
+ Hits                                          48317    48965     +648     
- Misses                                        14855    14899      +44     
Files with missing lines Coverage Δ
analysis/reanalyze/src/side_effects.ml 77.04% <100.00%> (+0.38%) ⬆️
analysis/src/completion_jsx.ml 75.00% <ø> (ø)
analysis/src/type_utils.ml 63.82% <100.00%> (ø)
analysis/src/xform.ml 88.14% <100.00%> (ø)
compiler/bsc/rescript_compiler_main.ml 71.49% <100.00%> (ø)
compiler/core/j.ml 100.00% <ø> (ø)
compiler/core/js_of_lam_variant.ml 57.89% <100.00%> (ø)
compiler/core/js_record_fold.ml 88.81% <100.00%> (+0.14%) ⬆️
compiler/core/js_record_iter.ml 95.49% <100.00%> (+0.08%) ⬆️
compiler/core/js_record_map.ml 95.75% <100.00%> (+0.07%) ⬆️
... and 84 more

... and 11 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.

Signed-off-by: Christoph Knittel <ck@cca.io>
@cristianoc

Copy link
Copy Markdown
Collaborator

Heads-up on a likely conflict with #8608 (merging the Lam IR into Lambda), and a suggestion on ordering.

Overlap: 26 compiler files. The part that matters is that this PR modifies seven files that #8608 deletes:

file changes here
compiler/core/lam.ml +9 / −14
compiler/core/lam_primitive.ml / .mli +13 / −4, +11 / −1
compiler/frontend/lam_constant.ml / .mli +8 / −3, +6 / −1
compiler/core/lam_convert.ml +5 / −4
compiler/core/lam_print.ml +3 / −2
compiler/core/lam_constant_convert.ml +2 / −2

About +57 / −31 in total. As far as I can tell those edits are all the same thing: propagating the changed Const_string payload through each layer of a representation that happened to be duplicated.

Suggestion: land #8608 first. The reason is that this duplication is exactly what it removes. After it, Lam_constant, Lam_primitive, Lam_print and the converters are gone, and the same change lands once in compiler/ml/lambda.ml and compiler/ml/printlambda.ml instead of eight times. The delete/modify conflicts resolve by taking the deletion and re-homing one small, well-understood change — so rebasing this PR on top should make it slightly smaller, not larger.

The reverse order looks worse: #8608 would have to carry this PR's in-flight string representation into files it is simultaneously deleting. Same work, harder direction. It would also cost that PR its main verification property — it claims generated JavaScript is unchanged, checked against runtime, Belt and the 620 modules in tests/tests, whereas this PR intentionally changes output (native template literals). Rebasing it onto this one means re-establishing that against a moved baseline, and any residual diff becomes ambiguous between the two changes.

One more thing worth knowing: compiler/ml/typecore.ml is the only file where both PRs make substantial edits (+81 / −69 here). #8608's change there is a single unrelated commit — a fix for a compiler crash on a polymorphic variant whose numeric name exceeds the int32 range — sitting next to the integer-literal decoding this PR reworks. That commit can be split out of #8608 into its own PR, which would leave the conflicts confined to the Lambda area. Happy to do that if it helps.

No urgency from my side — mostly flagging it early so the order is a decision rather than a surprise at rebase time.

@cknitt
cknitt force-pushed the codex/string-literal-representation branch from c292e07 to 513d122 Compare September 1, 2026 19:41
@cknitt

cknitt commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

@codex review

@pkg-pr-new

pkg-pr-new Bot commented Sep 1, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8606

@rescript/belt

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/belt@8606

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@8606

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@8606

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@8606

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@8606

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8606

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@8606

commit: cdf45ee

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 513d122385

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: Christoph Knittel <ck@cca.io>
Signed-off-by: Christoph Knittel <ck@cca.io>
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.

Normalize string literal representation to fix pattern matching

2 participants