diff --git a/dev/modules/template.md b/dev/modules/template.md index 460619a36..b6884f875 100644 --- a/dev/modules/template.md +++ b/dev/modules/template.md @@ -23,8 +23,9 @@ its test suite on PerlOnJava. | After Fix 7 | **3/106** | ~23/2884 | **2884** | XSLoader jar: shim overrides + stale .ttc cleanup | | After Fix 8 | **2/106** | ~3/2884 | **2884** | `use bytes` length fix for Latin-1 strings | | After Fix 9 | **1/106** | ~2/2884 | **2884** | Scope `packageExistsCache` sub entries to declaring package | +| After Fix 10 | **21/106** | 3/2642 | **2642** | `our`/`state` statement modifier fix (DESTROY regression: 20 tests) | -### Current: 105/106 passing (99%), 11 skipped, 1 truly failing (leak.t — DESTROY not implemented) +### Current: 85/106 passing (80%), 12 skipped, 21 failing (20 due to premature DESTROY — fix in progress on separate branch) --- @@ -83,23 +84,51 @@ XS module with a Perl parent. **File:** `src/main/java/org/perlonjava/runtime/perlmodule/XSLoader.java` +### Fix 10: `our`/`state` Statement Modifier Hoisting + +`Template::Service` (and 15 other TT files) declare `our $DEBUG = 0 unless defined $DEBUG`. +The parser's `handleStatementModifierWithMy` transformed this to `defined($DEBUG) || (our $DEBUG = 0)`, +placing the `$DEBUG` reference before the `our` declaration. Under `use strict`, this caused +"Global symbol requires explicit package name", preventing `Makefile.PL` from running at all. + +**Root cause:** The method only checked for `my` declarations (line 1046), not `our` or `state`. + +**Fix:** Extended the check in `StatementResolver.java` to handle `my`, `our`, and `state`: +``` +our $X = EXPR unless COND → (our $X, COND || ($X = EXPR)) +``` +This ensures the variable is declared before the condition is evaluated. + +**Files:** `src/main/java/org/perlonjava/frontend/parser/StatementResolver.java`, +`src/test/resources/unit/statement.t` + --- -## Remaining Failures (1/106 programs) +## Remaining Failures (21/106 programs) -### leak.t — 2/11 subtests failing (expected) +### Premature DESTROY — 20 test programs failing + +`Template::Context::DESTROY` sets `$self->{STASH} = undef` to break circular references. +Since DESTROY was implemented (commit `97ec12b8b`), PerlOnJava's refCount tracking triggers +DESTROY prematurely during `Template::_output()`, nullifying the stash while the Context is +still in use. This affects any test that calls `process()` with `INCLUDE`/`PROCESS` directives. + +**Status:** Fix in progress on a separate branch. + +### leak.t — subtests failing (expected) | Test | Failed | Total | Issue | |------|--------|-------|-------| -| leak.t | 2 | 11 | Tests 7, 11 — DESTROY not implemented in PerlOnJava (known limitation) | +| leak.t | varies | 11 | Premature DESTROY + original DESTROY timing tests | --- ## Next Steps -### Ready for merge -This branch is ready for review and merge. All actionable failures have been fixed. -The only remaining failure (`leak.t`) is due to a known PerlOnJava limitation (no DESTROY support). +### Premature DESTROY fix (separate branch) +The 20 failing tests are all caused by `Template::Context::DESTROY` firing while the Context +is still in use. A fix for PerlOnJava's refCount tracking is in progress on a separate branch. +Once merged, Template Toolkit should return to 105/106 passing. ### Post-merge: jcpan parallel test ordering When running `./jcpan --jobs 8 -t Template`, the compile tests (`compile2.t`, `compile3.t`, @@ -107,12 +136,6 @@ When running `./jcpan --jobs 8 -t Template`, the compile tests (`compile2.t`, `c to populate the compiled template cache. Running them sequentially (or with `-j 1`) always passes. This is a test-harness ordering issue, not a PerlOnJava bug. -### Not planned (known limitation) -- **leak.t tests 7, 11** — These tests verify that DESTROY is called when objects go out - of scope. PerlOnJava does not implement DESTROY (the JVM's tracing GC handles circular - references natively, making destructor-based cleanup unnecessary). These 2 subtests will - remain failing until/unless DESTROY support is added to PerlOnJava. - --- ## Progress Tracking @@ -150,9 +173,13 @@ passes. This is a test-harness ordering issue, not a PerlOnJava bug. - evalperl.t: 18/19 → **19/19** — RAWPERL blocks with illegal code now correctly eval'd - File: SubroutineParser.java - [x] Template tests: **105/106 passing** (1 failing, 11 skipped) — **99% pass rate** +- [x] Fix 10: `our`/`state` statement modifier hoisting in StatementResolver.java (2026-04-11) + - `our $DEBUG = 0 unless defined $DEBUG` now correctly hoists the declaration + - Unblocked Makefile.PL — Template Toolkit can now configure and build + - 20 tests regressed due to premature DESTROY (separate issue, separate branch) -### Remaining (not planned) -- [ ] leak.t tests 7, 11 — DESTROY not implemented +### Remaining +- [ ] Premature DESTROY — 20 tests failing (fix in progress on separate branch) --- diff --git a/src/main/java/org/perlonjava/frontend/parser/StatementResolver.java b/src/main/java/org/perlonjava/frontend/parser/StatementResolver.java index 2b281678f..89eaa52fe 100644 --- a/src/main/java/org/perlonjava/frontend/parser/StatementResolver.java +++ b/src/main/java/org/perlonjava/frontend/parser/StatementResolver.java @@ -1005,8 +1005,9 @@ public static void parseStatementTerminator(Parser parser) { } /** - * Handle statement modifiers (if/unless) with my declarations. + * Handle statement modifiers (if/unless) with my/our/state declarations. * For "my $x = EXPR if COND", the variable must be declared even when condition is false. + * For "our $x = EXPR unless COND", the variable must be declared before evaluating COND. * Uses comma operator to declare variable in current scope: (my $x, COND && ($x = EXPR)) * This avoids creating a new scope (which BlockNode would do). * @@ -1039,11 +1040,12 @@ private static Node handleStatementModifierWithMy(Node expression, Node modifier } } - // Check if expression is an assignment with 'my' on the left side + // Check if expression is an assignment with my/our/state on the left side if (expression instanceof BinaryOperatorNode assignNode && assignNode.operator.equals("=")) { Node left = assignNode.left; - // Check if left side is a 'my' declaration - if (left instanceof OperatorNode myNode && myNode.operator.equals("my")) { + // Check if left side is a 'my', 'our', or 'state' declaration + if (left instanceof OperatorNode myNode && + (myNode.operator.equals("my") || myNode.operator.equals("our") || myNode.operator.equals("state"))) { // Transform: my $x = EXPR if COND // Into: (my $x, COND && ($x = EXPR)) // The comma operator evaluates both in the current scope (no new scope created) diff --git a/src/test/resources/unit/statement.t b/src/test/resources/unit/statement.t index 6abf3448c..270c7380e 100644 --- a/src/test/resources/unit/statement.t +++ b/src/test/resources/unit/statement.t @@ -122,4 +122,39 @@ for my $i (1..3) { } is($scope_test, 12, 'scope in loops calculation'); +# our with statement modifier unless - variable must be declared before condition is evaluated +{ + package TestOurUnless; + use strict; + our $DEBUG = 0 unless defined $DEBUG; + ::is($DEBUG, 0, 'our $VAR = VAL unless defined $VAR - declares and assigns'); +} + +# our with statement modifier if +{ + package TestOurIf; + use strict; + our $FLAG = 1 if !defined $FLAG; + ::is($FLAG, 1, 'our $VAR = VAL if !defined $VAR - declares and assigns'); +} + +# our with statement modifier unless - pre-defined value preserved +{ + package TestOurPreDefined; + use strict; + $TestOurPreDefined::LEVEL = 42; + our $LEVEL = 0 unless defined $LEVEL; + ::is($LEVEL, 42, 'our $VAR = VAL unless defined $VAR - preserves pre-defined value'); +} + +# state with statement modifier unless +{ + use feature 'state'; + sub state_unless_test { + state $counter = 0 unless defined $counter; + return $counter; + } + is(state_unless_test(), 0, 'state $VAR = VAL unless defined $VAR - works'); +} + done_testing();