compiler: lower String relational ops to lexicographic comparison (string-wall slice 10, #458)#588
Merged
Merged
Conversation
…ring-wall slice 10) String `<`/`<=`/`>`/`>=` typecheck (the `comparison` branch returns Bool, #458) but the wasm backend lowered them to a signed-integer compare of the two `[len][utf8]` *pointers* — a meaningless ordering. This completes the string wall begun by slice 8b (`++`) and slice 9 (`==`/`!=`). The fix mirrors the slice-9 machinery exactly: - typecheck records String-typed relational nodes (string_rel_sites) by physical identity in synth's comparison branch; - elaborate_string_concat rewrites them to a new ExprStringRel AST node; - codegen lowers ExprStringRel to a byte-wise lexicographic comparison: compare the common prefix unsigned, and on a tie the shorter string is the smaller; the result (cmp in {-1,0,1}) maps onto `cmp <op> 0`. The loop is bounded at min(la,lb), so it never reads past either string. Type-blind passes get matching arms (interp/opt/effect_sites). Int/Float relational is untouched (only String operands are recorded). Verified: lexicographic ordering across decisive-byte / prefix-ordering / empty / equality / built-vs-literal cases; no regression across 41,215 int parity cases (incl. Maths 1982, VmState 38577) and slice-9 string `==`. New e2e fixture + test. https://claude.ai/code/session_01WoKhFQePiRsAj7aqnxbG8s
🔍 Hypatia Security ScanFindings: 40 issues detected
View findings[
{
"reason": "Action denoland/setup-deno@v2 needs attention",
"type": "unpinned_action",
"file": "publish-jsr.yml",
"action": "pin_sha",
"rule_module": "workflow_audit",
"severity": "medium"
},
{
"reason": "Issue in scorecard-enforcer.yml",
"type": "scorecard_publish_with_run_step",
"file": "scorecard-enforcer.yml",
"action": "split_scorecard_publish_job",
"rule_module": "workflow_audit",
"severity": "high"
},
{
"reason": "Issue in instant-sync.yml",
"type": "secret_action_without_presence_gate",
"file": "instant-sync.yml",
"action": "peter-evans/repository-dispatch",
"rule_module": "workflow_audit",
"severity": "high"
},
{
"reason": "Shell execution -- validate input before passing to shell (1 occurrences, CWE-78)",
"type": "js_exec_sync",
"file": "/home/runner/work/affinescript/affinescript/packages/affinescript-cli/mod.js",
"action": "flag",
"rule_module": "code_safety",
"severity": "high"
},
{
"reason": "Shell execution -- validate input before passing to shell (2 occurrences, CWE-78)",
"type": "js_exec_sync",
"file": "/home/runner/work/affinescript/affinescript/packages/affine-vscode/mod.js",
"action": "flag",
"rule_module": "code_safety",
"severity": "high"
},
{
"reason": "Shell execution -- validate input before passing to shell (1 occurrences, CWE-78)",
"type": "js_exec_sync",
"file": "/home/runner/work/affinescript/affinescript/affinescript-vite/src/affine-plugin-improved.js",
"action": "flag",
"rule_module": "code_safety",
"severity": "high"
},
{
"reason": "expect() in hot path (32 occurrences, CWE-754)",
"type": "expect_in_hot_path",
"file": "/home/runner/work/affinescript/affinescript/affinescriptiser/src/codegen/wasm_gen.rs",
"action": "flag",
"rule_module": "code_safety",
"severity": "medium"
},
{
"reason": "expect() in hot path (29 occurrences, CWE-754)",
"type": "expect_in_hot_path",
"file": "/home/runner/work/affinescript/affinescript/affinescriptiser/src/codegen/affine_gen.rs",
"action": "flag",
"rule_module": "code_safety",
"severity": "medium"
},
{
"reason": "unsafe block -- requires SAFETY comment (2 occurrences, CWE-676)",
"type": "unsafe_block",
"file": "/home/runner/work/affinescript/affinescript/runtime/src/panic.rs",
"action": "flag",
"rule_module": "code_safety",
"severity": "medium"
},
{
"reason": "unsafe block -- requires SAFETY comment (1 occurrences, CWE-676)",
"type": "unsafe_block",
"file": "/home/runner/work/affinescript/affinescript/runtime/src/alloc.rs",
"action": "flag",
"rule_module": "code_safety",
"severity": "medium"
}
]Powered by Hypatia Neurosymbolic CI/CD Intelligence |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
String
</<=/>/>=typecheck (thecomparisonbranch returnsBool, #458) but the wasm backend lowered them to a signed-integer compare of the two[len][utf8]pointers — a meaningless ordering. This completes the string wall: slice 8b fixed++, slice 9 fixed==/!=, and this slice fixes the four relational operators.Approach (mirrors slice 9 exactly)
synth'scomparisonbranch (string_rel_sites) — that branch only fires forOpLt/OpLe/OpGt/OpGe, so every recorded node is relationalelaborate_string_concatrewrites them to a newExprStringRelAST node (one walk now drives concat + eq + rel)ExprStringRelto a byte-wise lexicographic comparison: compare the common prefix unsigned (I32LtU/I32GtU), and on a tie the shorter string is the smaller; the resultcmp ∈ {−1,0,1}maps ontocmp <signed-op> 0. The loop is bounded atmin(la,lb)(viaSelect), so it never reads past either stringinterp/opt/effect_sites)Int/Floatrelational is untouched — only String operands are recorded.Verification
"ab" < "abc"), equality (<=/>=), empty strings (both directions), and built-vs-literal operands — a 10-case discriminating program returns the exact expected encoding==edge-case suitetest/e2e/fixtures/string_rel.affine) + test (test_e2e.ml, slice-10)Effect on the migration
With
++,==/!=, and now</<=/>/>=all lowering correctly, String operations work end-to-end on the wasm target. Future.res → .affinemigrations no longer need to integer-gate string comparison/ordering — the core driver of the "extract an integer brain, keep strings host-side" pattern is removed.https://claude.ai/code/session_01WoKhFQePiRsAj7aqnxbG8s
Generated by Claude Code