Repository navigation
Grounded Unicode identifiers in the lexer (XID_Start/XID_Continue ∪ Emoji) - #5653
Conversation
The DAG parser does not support // line comments anywhere — not at the top level between declarations, not inside list literals, not inside match bodies. Strip all // comments from unicode.dag, the XID test, and the v2 tokenizer so dsl_compile_clean_gate_passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Review finding is correct — three parallel copies of the same Unicode table violate §2/§3, and the v2
— sent from tidy-eagle-791 |
|
Fix: restored stale entries in v1_compiler_infer_method.rs The regen_stage0 WIP commit trimmed v1_compiler_infer_method.rs to match 04_method.dag exactly. But 04_method.dag is missing entries (doc_graph_orphan_count, inert_carrier_count, non_fold_residue_count, etc.) that CI claim tests still call — the stage0 was ahead of the source here. This exposed a pre-existing inconsistency. New commit a3a12e5: restore origin/main state of v1_compiler_infer_method.rs and add only the 3 XID entries on top. No change to 04_method.dag; the stale-registry cleanup is separate scope. — sent from tidy-eagle-791 |
|
Addressed all three review findings in commit b500843. Finding 1 & 2 (orphan generated files / extdeps_uri.rs compile bug): Removed extdeps_external_authority.rs, extdeps_uri.rs, v1_probe_emit_interp.rs, v1_test_non_ascii_perf_fixture.rs from GENERATED_STAGE0_FILES in regen_stage0.rs and deleted the files. These were artifacts of the WIP regen_stage0 run that generated modules not yet wired into lib.rs (and extdeps_uri.rs had the struct/variant name collision the review noted). Finding 3 (§3 single-authority on sentinel): Fixed properly — not just a comment.
cargo check -p v1-compiler --bins clean before push. New CI run: 28056747416. — sent from tidy-eagle-791 |
|
Review 32155 — addressed in 5ba02dd. The file was a stale artifact from before the v2→v1 compiler rename (#5028, efbdbb3). That rename moved the file to src/v1/stage0/src/coproduct_reflection.rs where it lives in main today. The old v2 path was never wired on this branch (no mod declaration, no use sites) and has now been removed. The minor note on is_reserved_emit_sentinel is acknowledged — that is the intended fail-closed behavior for round-trip sentinel collision avoidance, not a tokenizer bug. — sent from tidy-eagle-791 |
…tion narrowing Prove the keycap-base fix did not accidentally exclude the operator's actual target codepoints. Both glyphs have EmojiPresentation status (unlike # / * which have EmojiOtherAndEmojiComponent), so they correctly remain accepted as emoji ident chars and round-trip through the emit sentinel (_Eu1F7E2_ / _Eu1F7E1_). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clippy -D warnings caught .clone() on TokenShape (Copy type) in the smoke test. rustfmt reformatted three files that had drifted from the project style (unicode_xid_smoke.rs, v1_rt.rs, v1_compiler_tokenize.rs). No logic changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Review 32177 — both findings acknowledged, no code change needed. Finding 1 (sentinel_prefix_matches suffix length): Correct observation. The suffix check uses Finding 2 (apply_char_sanitization ordering): Also correct — Neither is a correctness break in the current corpus. Both noted as follow-on debt. — sent from tidy-eagle-791 |
The prior rust_tests run (#28065180602) hit sccache 'Failed to read response header' mid-compile of unicode-properties — infra flake, not a code issue. Retry via fresh run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… survives to_snake apply_char_sanitization was called before the naming-case transform, so the _Eu<UPPER-HEX>_ sentinel introduced for emoji codepoints was immediately fed into to_snake, which converts ASCII-uppercase chars (65-90) with underscore-prefix: E(65)→_e, U(85)→_u, F(70)→_f — producing __eu1_f600_ that the lexer's is_reserved_emit_sentinel check never matches. Fix: move apply_char_sanitization after the naming-case block. Emoji codepoints (>> 90) are outside the 65-90 range that to_snake/to_lower_char modify, so they pass through unchanged; the sentinel is introduced last and survives intact. Add emit_ident_sentinel_survives_case_conversion test discriminating the correct sentinel form _Eu1F600_ (accepted) from the mangled __eu1_f600_ (rejected). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in 7d7db2e. Root cause confirmed: Fix (option a): reordered Test added: — sent from tidy-eagle-791 |
Summary
Ground Unicode identifier classification on cited upstream authorities (UAX31 via `unicode-ident`, emoji via `unicode-properties`) rather than hand-rolled ASCII heuristics.
This grounds identifier acceptance in published Unicode standard (§3 single-authority: no internal taxonomy, cite the upstream spec). Operator target codepoints U+1F7E2 🟢 / U+1F7E1 🟡 are confirmed `EmojiPresentation` and survive the narrowing.
Changes
Stage0 hand-sync note (pre-existing §7 debt, not introduced here)
`v1_compiler_infer_method.rs` was hand-updated (not regenerated via `regen_stage0`). The formal fixed-point is unverifiable in this PR because `04_method.dag` and stage0 have a pre-existing drift on `origin/main`: 35 builtins are hand-extended into stage0 but absent from `04_method.dag`, meaning `regen_stage0` would drop them. This drift exists on `main` independent of this PR.
Verification: on `main`, `04_method.dag` has 68 entries and stage0 has 100. On this branch, the sets both grow by exactly 3 (`is_xid_start`, `is_xid_continue`, `is_emoji_ident` appear in both files; the drift set is identical to main). The 3 new builtins are correctly synced and are not in the drift set.
The pre-existing §7 fixed-point debt has been escalated separately by the reviewing manager.
Test plan