Add wildcard, function, loose equality and label to unsupported features#32
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds lexer/token and parser recognition for several previously-ignored syntactic forms (function declarations/expressions, loose equality Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
frostney
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/language-restrictions.md (3)
160-164: Add language specifier to fenced code block.The warning example code block should specify a language (e.g.,
textorplaintext) to satisfy markdown linting rules and improve consistency with other documentation.📝 Suggested fix
-``` +```text Warning: 'function' declarations are not supported in GocciaScript Suggestion: Use arrow functions instead: const name = (...) => { ... } --> script.js:1:1</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/language-restrictions.mdaround lines 160 - 164, Update the fenced code
block that contains the warning text starting with "Warning: 'function'
declarations are not supported in GocciaScript" to include a language specifier
(for example changetotext) so the block becomestext ...; this
satisfies markdown linting and keeps the example consistent with other docs.</details> --- `184-188`: **Add language specifier to fenced code block.** Similar to the function keyword warning block, this should specify `text` or `plaintext` for consistency. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text Warning: '==' (loose equality) is not supported in GocciaScript Suggestion: Use '===' (strict equality) instead --> script.js:1:10 ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/language-restrictions.mdaround lines 184 - 188, Update the fenced code
block that shows the warning about '==' to include a language specifier (e.g.,
change the opening fence fromtotext or ```plaintext) so it matches the
other examples like the "function keyword warning" block; locate the block
containing the lines "Warning: '==' (loose equality) is not supported in
GocciaScript" and add the language token to the opening fence for consistency.</details> --- `270-273`: **Add language specifier to fenced code block.** The labeled statement warning example should also specify a language. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text Warning: Labeled statements are not supported in GocciaScript --> script.js:1:1 ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/language-restrictions.mdaround lines 270 - 273, Update the fenced code
block containing the warning text "Warning: Labeled statements are not supported
in GocciaScript --> script.js:1:1" to include a language specifier (e.g.,docs/language-restrictions.md file around the example and change the opening fence from ``` to ```text so the warning example is explicitly marked as plain text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/language-restrictions.md`:
- Around line 160-164: Update the fenced code block that contains the warning
text starting with "Warning: 'function' declarations are not supported in
GocciaScript" to include a language specifier (for example change ``` to
```text) so the block becomes ```text ... ```; this satisfies markdown linting
and keeps the example consistent with other docs.
- Around line 184-188: Update the fenced code block that shows the warning about
'==' to include a language specifier (e.g., change the opening fence from ``` to
```text or ```plaintext) so it matches the other examples like the "function
keyword warning" block; locate the block containing the lines "Warning: '=='
(loose equality) is not supported in GocciaScript" and add the language token to
the opening fence for consistency.
- Around line 270-273: Update the fenced code block containing the warning text
"Warning: Labeled statements are not supported in GocciaScript -->
script.js:1:1" to include a language specifier (e.g., ```text) for proper
rendering; locate the block in the docs/language-restrictions.md file around the
example and change the opening fence from ``` to ```text so the warning example
is explicitly marked as plain text.
Contributor
Benchmark Results179 benchmarks · 🟢 92 improved · 87 unchanged · avg +9.8% arrays.js — 🟢 1 improved, 18 unchanged · avg +4.0%
classes.js — 15 unchanged · avg +2.8%
closures.js — 11 unchanged · avg +5.4%
collections.js — 🟢 5 improved, 7 unchanged · avg +7.3%
destructuring.js — 🟢 1 improved, 13 unchanged · avg +5.3%
fibonacci.js — 🟢 1 improved, 5 unchanged · avg +5.0%
iterators.js — 🟢 10 improved, 10 unchanged · avg +6.6%
json.js — 🟢 14 improved, 6 unchanged · avg +8.2%
jsx.jsx — 🟢 21 improved · avg +19.1%
numbers.js — 🟢 10 improved, 1 unchanged · avg +11.8%
objects.js — 🟢 7 improved · avg +19.3%
promises.js — 🟢 12 improved · avg +23.5%
strings.js — 🟢 10 improved, 1 unchanged · avg +12.3%
Measured on ubuntu-latest x64. Changes within ±7% are considered insignificant. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/language-restrictions.md (1)
182-190: Clarify whether operands are evaluated at runtime.Line 190 states "Both operands are parsed but discarded." The phrase "parsed but discarded" is ambiguous—it's unclear whether the operands are evaluated at runtime or skipped entirely.
This matters for side effects:
let x = 0; if (x++ == x++) { } // Are the increments executed? if (foo() == bar()) { } // Are foo() and bar() called?Consider clarifying whether:
- Operands are not evaluated at runtime (skipped entirely), or
- Operands are evaluated (side effects happen) but the comparison result is discarded
📝 Suggested clarification
Replace line 190 with one of:
If operands are NOT evaluated:
Both operands are parsed but not evaluated at runtime (no side effects). Because the expression evaluates to `undefined` (which is falsy), `==`/`!=` in conditions (e.g., `if (a == b)`) will never enter the truthy branch.If operands ARE evaluated:
Both operands are evaluated (side effects occur) but the comparison result is discarded. The expression evaluates to `undefined`. Because `undefined` is falsy, `==`/`!=` in conditions (e.g., `if (a == b)`) will never enter the truthy branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/language-restrictions.md` around lines 182 - 190, The sentence "Both operands are parsed but discarded." in the paragraph about `==`/`!=` is ambiguous about side effects—update that sentence to explicitly state whether the operand expressions are evaluated at runtime or not; replace it with one of the two suggested clarifications from the reviewer: either "Both operands are parsed but not evaluated at runtime (no side effects)...", or "Both operands are evaluated (side effects occur) but the comparison result is discarded..."; keep the surrounding examples (`if (a == b)`, `if (x++ == x++)`, `if (foo() == bar())`) intact so readers can see the semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/language-restrictions.md`:
- Around line 182-190: The sentence "Both operands are parsed but discarded." in
the paragraph about `==`/`!=` is ambiguous about side effects—update that
sentence to explicitly state whether the operand expressions are evaluated at
runtime or not; replace it with one of the two suggested clarifications from the
reviewer: either "Both operands are parsed but not evaluated at runtime (no side
effects)...", or "Both operands are evaluated (side effects occur) but the
comparison result is discarded..."; keep the surrounding examples (`if (a ==
b)`, `if (x++ == x++)`, `if (foo() == bar())`) intact so readers can see the
semantics.
This was referenced Apr 4, 2026
This was referenced Apr 23, 2026
frostney
added a commit
that referenced
this pull request
May 5, 2026
ES2026 §11.2 WhiteSpace and §11.3 LineTerminator include Unicode characters beyond ASCII space — Tab/LF/CR/VT/FF/SP/NBSP/USP plus the ZWNBSP and LS/PS line terminators. parseInt/parseFloat call TrimString(string, start) which uses StrWhiteSpace, so a Unicode whitespace prefix like `\u3000` (ideographic space) must be skipped before scanning digits. Goccia's parseInt/parseFloat were calling FreePascal's `Trim()`, which only strips ASCII whitespace (#0..#32). `parseInt("\u20001")` returned NaN instead of 1. Switch both to `TrimECMAScriptWhitespace` from `TextSemantics`, the same helper String.prototype.trim and BigInt parsing already use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
frostney
added a commit
that referenced
this pull request
May 6, 2026
* Add --compat-traditional-for-loop opt-in flag Adds traditional C-style for(init; test; update) loops behind a new compatibility flag, mirroring the existing --compat-var / --compat-function posture. let/const declarations in for-init create per-iteration lexical environments per ES2026 §14.7.4.4 so closure capture pins per iteration. var declarations require both --compat-var and the new flag, hoist out of the loop, and share a single binding. Bytecode mode includes a counted-loop fast path mirroring CompileCountedForOf for for(let i = N; i <op> M; i++|i--) shapes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire --compat-traditional-for-loop through GocciaScriptLoaderBare The bare loader has its own option-parsing and engine-config path, so the flag has to be threaded through it explicitly (mirrors the existing --compat-var / --compat-function plumbing). Without this, test262 (which runs through the bare loader with --compat-all) was not picking up the flag and only one extra traditional-for test passed across the suite. With the wiring, language/statements/for/ goes from 60/385 to 325/385 in bytecode mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR review: fast-path safety, const update, interpolation parser - TryCompileCountedFor now bails when the init has a type annotation or the condition RHS isn't a literal/identifier the body doesn't reassign; the spec re-evaluates the test each iteration, so snapshotting a side-effecting or mutable RHS once is wrong (#531 review). - EvaluateFor's iteration→header sync now uses ForceUpdateBinding so a body that runs to completion under `for (const ...)` doesn't trip the evaluator's own writability check on the next iteration's snapshot. - ParseInterpolationExpression threads TraditionalForLoopsEnabled into child parsers so an IIFE inside a template `${...}` can use traditional for when the surrounding source has it enabled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Gate ParseTraditionalForBody on top-level ';' in for-header Fixes a regression caused by my own dispatch — `for (lhs of expr)` heads where `lhs` is an assignment target (not a declaration) were previously warned-and-skipped, but now fell through to ParseTraditionalForBody and syntax-errored on `Expected ';'` before the `of`. Same shape: `for ([a, b] of arr)`, `for ({x} of objs)`, etc. Several test262 suites use these forms in harness/feature tests. LooksLikeTraditionalForHeader is a pure peek that returns True only when a top-depth ';' exists inside the for-header parens. for-of/for-in heads contain no top-level ';', so they fall back to the warn-and-skip path. Also handles the edge case in language/statements/for/head-init-async-of.js where `for (async of => {}; ...; ...)` is a traditional for whose init is an arrow with parameter named `of` — the prior heuristic would have misclassified that as a for-of shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Decouple --compat-traditional-for-loop from --compat-all Bundling traditional `for(;;)` into --compat-all re-runs loop bodies that warn-and-skipped on baseline, surfacing 367 unrelated engine gaps (Atomics, Intl, Math.pow edge cases, RegExp tests) that were trivially passing before. Test262 runs --compat-all unconditionally through the bare loader, so the bundling forced a -31 net merge-blocking regression. Make the flag strictly opt-in: callers who want traditional `for(;;)` must pass --compat-traditional-for-loop explicitly (or set the config / engine property). --compat-all continues to OR in --compat-var and --compat-function as before. Test262 baseline is restored. Also tighten TryCompileCountedFor's condition-RHS guard to literals only (#531 review): `ForBodyAssignsIdentifier` doesn't see writes through IIFEs/callbacks/property setters, so `i < limit` was unsafe to fast-path even when the body looked clean. Falls through to the general path where the condition is re-evaluated each iteration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "Decouple --compat-traditional-for-loop from --compat-all" This reverts commit cd0f7bb. * Re-tighten counted-for fast path RHS to literal-only Brings the bot review fix forward over the decouple revert. The identifier-RHS branch in TryCompileCountedFor used ForBodyAssignsIdentifier as a safety check, but that walker doesn't see writes through IIFEs, callbacks, property setters, or eval-like reaches — so a bare-identifier RHS was unsafe for `i < limit` shapes even when the body looked clean. Falls through to the general path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Implement Math.pow & parseFloat per ES2026 spec Both bugs were silently passing test262 because traditional for(;;) loops in their test bodies warn-and-skipped on baseline. Restoring --compat-all coverage in #531 surfaced both as honest regressions; fix at the source. Math.pow (§6.1.6.1.3 Number::exponentiate) Previous impl forwarded directly to FreePascal's `Power(B, E)`, which uses `exp(E * ln(B))` and returns NaN for any base ≤ 0. Spec has 30+ special cases: NaN/+0/-0 exponent, ±∞ base, ±∞ exponent, signed-zero/Infinity sign rules driven by "exponent is an odd integer". Adds explicit branches for §6.1.6.1.3 steps 1-13 and guards Round() against doubles above 2^53 (oddness can only be detected up to that magnitude — Round(1.8e308) overflows Int64). Fixes Math/pow/* tests for ±Infinity^x and x^±Infinity. parseFloat (§21.1.2.13.1 StrUnsignedDecimalLiteral) Previous impl parsed integer + fractional digits but no ExponentPart, so `parseFloat("0.1e1")` returned 0.1 instead of 1. Adds optional `(e|E)[+-]?DecimalDigits` parsing after the fraction; malformed exponent (stray 'e' with no digits) returns the mantissa alone per the longest-valid-prefix rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * parseInt/parseFloat trim ECMAScript whitespace, not just ASCII ES2026 §11.2 WhiteSpace and §11.3 LineTerminator include Unicode characters beyond ASCII space — Tab/LF/CR/VT/FF/SP/NBSP/USP plus the ZWNBSP and LS/PS line terminators. parseInt/parseFloat call TrimString(string, start) which uses StrWhiteSpace, so a Unicode whitespace prefix like `\u3000` (ideographic space) must be skipped before scanning digits. Goccia's parseInt/parseFloat were calling FreePascal's `Trim()`, which only strips ASCII whitespace (#0..#32). `parseInt("\u20001")` returned NaN instead of 1. Switch both to `TrimECMAScriptWhitespace` from `TextSemantics`, the same helper String.prototype.trim and BigInt parsing already use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Number.prototype.toString radix + Math.ceil signed-zero per spec Number.prototype.toString (§21.1.3.6 / §6.1.6.1.20) Previous impl only handled radix 10 (default ToString) and radix 16 (IntToHex two's-complement, which gave `(-255).toString(16) = 'ffffffffffffff01'` instead of `-ff`). Every radix in 11-15, 17-36 fell through to decimal output. Implements the generalized decimal-to-radix algorithm: optional sign + integer-part-digits + optional `.fraction-digits`, using `0-9a-z` for digits 10-35. IntegerToRadixString and FractionToRadixString are file-local helpers shared with the integer path; the fraction conversion uses 52 digits (enough to round-trip a binary64 mantissa) with trailing-zero trimming. Fixes Number/prototype/toString/a-z and any test using non-10 / non-16 radix. Math.ceil (§21.3.2.10) Per spec step 3, when the argument is in (-1, 0) the result is -0𝔽 (preserves signed-zero). Goccia returned `+0` because it skipped straight to FreePascal's Ceil(), which collapses signed zeros. Adds the explicit "negative-fractional → -0" branch so `Math.ceil(-0.1) === -Math.floor(0.1)` per Object.is. Fixes Math/ceil/S15.8.2.6_A7 (the 2000-iteration ceil(x) ≡ -floor(-x) identity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Math.floor preserves signed zero per spec ES2026 §21.3.2.16: Math.floor(-0𝔽) is -0𝔽, not +0𝔽. Goccia was passing through to FreePascal's `Floor()` which collapses signed zeros. Returns the argument directly when it is -0 or +0 so the ceil(x) ≡ -floor(-x) identity (Math.floor/S15.8.2.6_A7) holds at the zero boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Array.{sort,toSorted}: validate comparator type/order per spec Both methods had two divergences from ES2026 §23.1.3.{29,34}: 1. Validation order: spec step 1 throws TypeError when comparefn is non-undefined and non-callable, BEFORE LengthOfArrayLike reads `this.length`. Goccia read length first, so a getter that throws on length was observed even when the comparator argument was already bad. test262 specifically covers this with a `getLengthThrow` object plus invalidComparators list (null/true/false/""/regex/number/ bigint/[]/{}/Symbol). 2. Error type: ThrowError was throwing a generic Error rather than TypeError, so `assert.throws(TypeError, ...)` saw the wrong constructor. Switch to ThrowTypeError to match the spec's "throw a TypeError exception". Also pull comparator extraction up so the undefined-comparator branch falls through to the default ascending compare without re-checking AArgs.Length downstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * parseFloat: use division for negative exponents (FP precision) `parseFloat("0.1e-1")` returned `0.010000000000000002` instead of `0.01`. Multiplying 0.1 by IntPower(10, -1) = 0.1 yields the imprecise product (0.1 * 0.1 has FP error). Dividing by IntPower(10, 1) = 10 lands exactly on the closest IEEE 754 double to 0.01. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PR review: tighten counted fast path + fix Number.toString edge cases - TryCompileCountedFor now also rejects non-integer literal RHS so `for(let i=0; i<3.5; i++)` doesn't take the OP_GTE_INT/OP_LTE_INT fast path; falls through to the general path which preserves IEEE 754 compare semantics. (#531 review) - IntegerToRadixString takes a Double, so values above Int64.MaxValue (which still produce a finite Number) no longer overflow on Round() and stringify via repeated div/mod in floating-point. The trailing-bit alignment for values past 2^53 is implementation-defined per ES2026 §6.1.6.1.20. - FractionToRadixString sizes the digit budget per radix as `Ceil(53 / Log2(R)) + 4` so binary64-precision values capture their mantissa in any base 2..36 (the previous fixed 52-digit cap was too short for radix 2 and overshot for radix 36). Full subnormal range remains future work — a proper Steele-White / Grisu implementation is tracked separately. - docs/language.md: clarify `continue` only applies to iteration constructs, never `switch`. - docs/decision-log.md: spell out that `--compat-traditional-for-loop` is bundled into `--compat-all` and reference the engine-gap issues surfaced (#540 / #541 / #542). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PR review: Int() in Number.toString + tighter no-op doc wording - IntegerToRadixString and the call site at NumberToString now use Int() instead of Trunc(). Trunc() returns Int64 and tripped a range-check error for any double past 2^63 (e.g. (1e100).toString(16)). Int() returns the truncated value as Double so arbitrarily large finite numbers stringify; Round() narrows just the small modulo digit. (#531 review) - docs/language.md: replace "loops" in the Graceful Handling bullet with the precise excluded list — `while`/`do...while` always, and traditional `for(;;)` only when --compat-traditional-for-loop is off. The previous wording implied all loops are no-op which now conflicts with opt-in traditional-for support. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PR review: align downstream "traditional loops excluded" wording Two follow-on doc consistencies after opt-in traditional `for(;;)`: - docs/language.md § Labeled Statements: "Since GocciaScript excludes traditional loops, labels serve no purpose" was outdated — traditional for(;;) is now available behind --compat-traditional-for-loop. Rewrite to call out which loop forms exist and that labeled break/continue isn't implemented for any of them. - docs/language-tables.md while/do...while row: list traditional for(;;) (with --compat-traditional-for-loop) alongside for-of and array methods as the suggested alternatives, matching the prose in language.md § while and do...while. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Move for-loop var tests to sibling dir for explicit separation `tests/language/for-loop/with-var/` was a nested subdir; the goccia.json inside it correctly only applied to its own tests, so isolation was empirically fine. But the layout made it look like compat-var was a sub-concern of for-loop, when it's really an orthogonal flag combination. Move to a sibling at `tests/language/for-loop-var/` so the directory tree makes the concern split obvious: tests/language/for-loop/ — only --compat-traditional-for-loop (let/const tests, default features) tests/language/for-loop-var/ — --compat-traditional-for-loop + --compat-var (combination tests) Each dir's goccia.json controls its own scope; running either, both, or neither still gives the same per-test config.
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
1 participant
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 by CodeRabbit
New Features
Documentation
Tests