Add support for class static blocks#264
Merged
Merged
Conversation
- Parse, compile, and execute `static { ... }` class blocks
- Add runtime coverage for class initialization order and `this` binding
- Document static blocks as a supported language feature
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ES2022 class static block support: parser recognizes Changes
Sequence Diagram(s)sequenceDiagram
participant Parser
participant AST
participant Compiler
participant Bytecode
participant VM
participant Evaluator
Parser->>AST: Parse class body -> emit cekStaticBlock element (BlockBody)
AST->>Compiler: Provide class elements in source order
Compiler->>Bytecode: CompileStaticBlock -> emit OP_CLOSURE + OP_CLASS_EXEC_STATIC_BLOCK
Bytecode->>VM: Run class-definition bytecode
VM->>Evaluator: OP_CLASS_EXEC_STATIC_BLOCK -> invoke closure with this=class
Evaluator->>Evaluator: ExecuteStaticBlock statements in init scope (this bound)
Evaluator-->>VM: Return/complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
units/Goccia.Compiler.Statements.pas (1)
2111-2142:⚠️ Potential issue | 🟠 MajorPreserve source-order interleaving for static fields and static blocks.
These two passes still emit all static field initializers first and only then execute
cekStaticBlockelements. That breaks the advertised source-order semantics for cases likeclass C { static { this.x = 1 } static y = this.x }, whereyshould see the block’s side effect. The static initializer path needs to be driven fromClassDef.FElementsin a single ordered pass instead of mixing map iteration for fields with a later block-only pass.Based on PR objectives, static blocks are expected to execute in source order.
Also applies to: 2279-2310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Compiler.Statements.pas` around lines 2111 - 2142, The current code emits all static field initializers (ClassDef.StaticProperties / ClassDef.PrivateStaticProperties and CompileComputedElements) and only afterward emits static blocks (CompileStaticBlock), breaking source-order semantics; change the logic to iterate ClassDef.FElements once in order and for each element switch on its Kind: when it's a static field/public property, allocate register, CompileExpression for its Value, add constant name (use same '#' prefix for private), emit the OP_SET_PROP_CONST (and OP_CLASS_DECLARE_PRIVATE_STATIC_CONST for private), free the register; when it's a computed element call CompileComputedElements for that single element or inline its handling; when it's cekStaticBlock call CompileStaticBlock immediately; remove the separate map-based static-properties loops and the later static-block-only pass so initializers and blocks are emitted in source order (references: ClassDef.FElements, ClassDef.StaticProperties, ClassDef.PrivateStaticProperties, CompileComputedElements, CompileStaticBlock, OP_SET_PROP_CONST, OP_CLASS_DECLARE_PRIVATE_STATIC_CONST).
🧹 Nitpick comments (3)
units/Goccia.Evaluator.pas (1)
1744-1744: Use current spec edition year in the annotation.Line 1744 uses
ES2022; this file otherwise follows current-editionESYYYYtags.As per coding guidelines "When implementing ECMAScript-specified behavior, annotate each function or method with a comment referencing the relevant spec section using the format
// ESYYYY §X.Y.Z ...whereYYYYis the current edition year."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Evaluator.pas` at line 1744, Replace the hard-coded "ES2022" in the comment annotating "ClassStaticBlockDefinition: execute static block body" with the current ECMAScript edition tag used elsewhere in the file (i.e., change ES2022 to the same ESYYYY year used by the other ESYYYY annotations) so the comment matches the repository's current-spec-year convention.tests/language/classes/static-blocks.js (2)
95-108: Coversuperinside the inheritance case too.This currently proves base-before-derived execution order, but not the inheritance-specific lookup semantics that usually break first. Adding one
superread inside the child static block would make this case much more valuable.Suggested test strengthening
test("static blocks with inheritance", () => { const order = []; class Base { + static baseValue = 1; static { order.push("Base"); } } class Child extends Base { static { order.push("Child"); + this.inherited = super.baseValue; } } expect(order).toEqual(["Base", "Child"]); + expect(Child.inherited).toBe(1); });As per coding guidelines,
JavaScript end-to-end tests are the primary way of testing GocciaScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/language/classes/static-blocks.js` around lines 95 - 108, The test "static blocks with inheritance" currently only verifies execution order for class Base and Child; modify the Child static block to perform a read of super (e.g., access super.someProperty or call super in a harmless way) so the test also exercises inheritance lookup semantics during static initialization—locate the Child class static block in the test and add a benign super read to ensure superclass property resolution is triggered while preserving the existing order assertion on the order array.
239-246: This doesn't actually test the named class-expression binding.
this.value = 42only shows the static block executed. If named class expressions are part of the feature surface, the assertion should prove thatNamedis visible inside the block and aliases the constructor.Suggested assertion change
test("named class expression with static block", () => { const MyClass = class Named { static { - this.value = 42; + this.value = Named === this; } }; - expect(MyClass.value).toBe(42); + expect(MyClass.value).toBe(true); });As per coding guidelines,
JavaScript end-to-end tests are the primary way of testing GocciaScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/language/classes/static-blocks.js` around lines 239 - 246, The test currently only verifies the static block ran (this.value = 42) but does not prove the named class-expression binding; update the test "named class expression with static block" so the static block uses the inner name Named to set a property showing it aliases the constructor (for example set a flag/property on this when Named === this) and then assert that MyClass has that property true; locate the class expression (const MyClass = class Named { static { ... } };) and modify the static block and final expect to assert the Named binding equals the class constructor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.Evaluator.pas`:
- Around line 2008-2014: The static blocks are executed in a separate pass after
static-property processing, breaking source-order semantics; fix by executing
static blocks inline during the same single pass over AClassDef.FElements so
they interleave with static field definitions in source order. Locate the loop
that iterates AClassDef.FElements for static properties and, instead of
deferring cekStaticBlock handling to the later loop, call
ExecuteStaticBlock(AClassDef.FElements[I].StaticBlockBody, AContext, ClassValue)
immediately when AClassDef.FElements[I].Kind = cekStaticBlock; remove the
separate post-pass that currently runs static blocks.
In `@units/Goccia.Parser.pas`:
- Around line 2949-2964: The parser currently assigns MemberDecorators to a
static block element (when IsStatic and Check(gttLeftBrace) branch creates an
element with Kind := cekStaticBlock), but ECMAScript forbids decorators on class
static blocks; update that branch to detect if MemberDecorators is non-empty and
raise a TGocciaSyntaxError (with an appropriate message) instead of assigning
Decorators and creating the static block element; locate the block that sets
Elements[High(Elements)].Kind := cekStaticBlock and add the guard before copying
MemberDecorators/StaticBlockBody (or clear MemberDecorators) so static blocks
are rejected when decorators are present.
---
Outside diff comments:
In `@units/Goccia.Compiler.Statements.pas`:
- Around line 2111-2142: The current code emits all static field initializers
(ClassDef.StaticProperties / ClassDef.PrivateStaticProperties and
CompileComputedElements) and only afterward emits static blocks
(CompileStaticBlock), breaking source-order semantics; change the logic to
iterate ClassDef.FElements once in order and for each element switch on its
Kind: when it's a static field/public property, allocate register,
CompileExpression for its Value, add constant name (use same '#' prefix for
private), emit the OP_SET_PROP_CONST (and OP_CLASS_DECLARE_PRIVATE_STATIC_CONST
for private), free the register; when it's a computed element call
CompileComputedElements for that single element or inline its handling; when
it's cekStaticBlock call CompileStaticBlock immediately; remove the separate
map-based static-properties loops and the later static-block-only pass so
initializers and blocks are emitted in source order (references:
ClassDef.FElements, ClassDef.StaticProperties, ClassDef.PrivateStaticProperties,
CompileComputedElements, CompileStaticBlock, OP_SET_PROP_CONST,
OP_CLASS_DECLARE_PRIVATE_STATIC_CONST).
---
Nitpick comments:
In `@tests/language/classes/static-blocks.js`:
- Around line 95-108: The test "static blocks with inheritance" currently only
verifies execution order for class Base and Child; modify the Child static block
to perform a read of super (e.g., access super.someProperty or call super in a
harmless way) so the test also exercises inheritance lookup semantics during
static initialization—locate the Child class static block in the test and add a
benign super read to ensure superclass property resolution is triggered while
preserving the existing order assertion on the order array.
- Around line 239-246: The test currently only verifies the static block ran
(this.value = 42) but does not prove the named class-expression binding; update
the test "named class expression with static block" so the static block uses the
inner name Named to set a property showing it aliases the constructor (for
example set a flag/property on this when Named === this) and then assert that
MyClass has that property true; locate the class expression (const MyClass =
class Named { static { ... } };) and modify the static block and final expect to
assert the Named binding equals the class constructor.
In `@units/Goccia.Evaluator.pas`:
- Line 1744: Replace the hard-coded "ES2022" in the comment annotating
"ClassStaticBlockDefinition: execute static block body" with the current
ECMAScript edition tag used elsewhere in the file (i.e., change ES2022 to the
same ESYYYY year used by the other ESYYYY annotations) so the comment matches
the repository's current-spec-year convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6e77bc36-bb16-4264-8a6b-475780bf6995
📒 Files selected for processing (9)
docs/language-restrictions.mdtests/language/classes/static-blocks.jsunits/Goccia.AST.Statements.pasunits/Goccia.Bytecode.OpCodeNames.pasunits/Goccia.Bytecode.pasunits/Goccia.Compiler.Statements.pasunits/Goccia.Evaluator.pasunits/Goccia.Parser.pasunits/Goccia.VM.pas
Contributor
Benchmark Results324 benchmarks Interpreted: 🟢 44 improved · 🔴 233 regressed · 47 unchanged · avg -3.1% arraybuffer.js — Interp: 🔴 12, 2 unch. · avg -5.2% · Bytecode: 🟢 4, 🔴 7, 3 unch. · avg -2.9%
arrays.js — Interp: 🔴 19 · avg -5.1% · Bytecode: 🟢 1, 🔴 12, 6 unch. · avg -1.9%
async-await.js — Interp: 🔴 6 · avg -5.2% · Bytecode: 🔴 4, 2 unch. · avg -2.0%
base64.js — Interp: 🟢 1, 🔴 9 · avg -6.3% · Bytecode: 🟢 1, 🔴 7, 2 unch. · avg -4.7%
classes.js — Interp: 🔴 27, 4 unch. · avg -4.5% · Bytecode: 🔴 16, 15 unch. · avg -5.2%
closures.js — Interp: 🔴 11 · avg -4.8% · Bytecode: 🟢 3, 🔴 4, 4 unch. · avg -0.3%
collections.js — Interp: 🟢 2, 🔴 10 · avg -7.2% · Bytecode: 🟢 5, 🔴 3, 4 unch. · avg +0.5%
destructuring.js — Interp: 🟢 7, 🔴 9, 6 unch. · avg -1.0% · Bytecode: 🔴 19, 3 unch. · avg -4.0%
fibonacci.js — Interp: 🔴 7, 1 unch. · avg -5.3% · Bytecode: 🟢 1, 🔴 6, 1 unch. · avg -3.3%
for-of.js — Interp: 🔴 2, 5 unch. · avg -0.9% · Bytecode: 🟢 1, 🔴 3, 3 unch. · avg -3.5%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 12, 🔴 20, 10 unch. · avg -0.5% · Bytecode: 🟢 4, 🔴 33, 5 unch. · avg -5.2%
json.js — Interp: 🔴 20 · avg -6.1% · Bytecode: 🟢 4, 🔴 8, 8 unch. · avg -1.5%
jsx.jsx — Interp: 🟢 5, 🔴 10, 6 unch. · avg -0.7% · Bytecode: 🟢 4, 🔴 11, 6 unch. · avg -3.2%
modules.js — Interp: 🔴 9 · avg -4.5% · Bytecode: 🟢 1, 🔴 8 · avg -3.8%
numbers.js — Interp: 🔴 11 · avg -6.0% · Bytecode: 🟢 5, 🔴 5, 1 unch. · avg -2.8%
objects.js — Interp: 🟢 2, 🔴 4, 1 unch. · avg +0.2% · Bytecode: 🟢 1, 🔴 3, 3 unch. · avg +0.3%
promises.js — Interp: 🔴 5, 7 unch. · avg -1.1% · Bytecode: 🔴 8, 4 unch. · avg -2.2%
regexp.js — Interp: 🔴 11 · avg -4.4% · Bytecode: 🟢 2, 🔴 5, 4 unch. · avg -0.5%
strings.js — Interp: 🔴 11 · avg -6.8% · Bytecode: 🟢 1, 🔴 8, 2 unch. · avg -2.5%
typed-arrays.js — Interp: 🟢 9, 🔴 10, 3 unch. · avg -0.0% · Bytecode: 🔴 20, 2 unch. · avg -9.2%
uint8array-encoding.js — Interp: 🟢 6, 🔴 10, 2 unch. · avg +0.6% · Bytecode: 🟢 4, 🔴 11, 3 unch. · avg -5.5%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
…ic blocks Address CodeRabbit review feedback: - Static fields and static blocks now execute in source order via a unified FElements pass (fixes interleaving semantics per ES2022 §15.7.14) - Parser rejects decorators on static blocks with a SyntaxError - Parser always adds static fields to FElements for source-order tracking - HasDecorators check no longer falsely triggers for undecorated static fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coderabbitai
Bot
added
the
documentation
Use the suggestion-based error pattern (SSuggestStaticBlockNoDecorators) so the error output includes the standard "Suggestion:" line. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
static { ... }class blocks.thisbound to the class constructor, including support for inheritance and class expressions.OP_CLASS_EXEC_STATIC_BLOCKopcode.Testing
tests/language/classes/static-blocks.jscovering execution order,thisbinding, static fields/methods, private static fields, inheritance, and class expressions.docs/language-restrictions.mdto document static blocks as supported syntax.