Skip to content

Fix: goto &sub in use/import not executing target subroutine#320

Merged
fglock merged 36 commits into
masterfrom
fix/goto-tailcall-import
Mar 15, 2026
Merged

Fix: goto &sub in use/import not executing target subroutine#320
fglock merged 36 commits into
masterfrom
fix/goto-tailcall-import

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Mar 15, 2026

Summary

When a module's import method uses goto &other::import, the TAILCALL marker was being discarded by the parseUseDeclaration code path. This caused Moo::Role (which does goto &Role::Tiny::import) to fail to export has, with, requires etc.

Root Cause

The RuntimeCode.apply() call in StatementParser.parseUseDeclaration() was discarding the return value. When the import method returns a TAILCALL marker (from goto &sub), it needs to be handled with a trampoline loop.

Fix

Added TAILCALL trampoline loop in StatementParser.parseUseDeclaration() to handle tail calls in import methods, matching the pattern used in WarnDie.java and other call sites.

Additional Fixes (jcpan/CPAN.pm improvements)

  • VersionHelper.java: Handle "undef" and non-numeric version strings gracefully (fixes "For input string: undef" crashes)
  • MakeMaker.pm: Load ExtUtils::MM so MM->parse_version() works (fixes "Error while parsing version number" warnings)
  • Sub::Util: New Java implementation (SubUtil.java) with set_subname, subname, prototype, set_prototype (required by Moo)
  • Scalar::Util, List::Util: Added $VERSION to Java implementations + XSLoader stub .pm files for CPAN.pm version detection
  • Test::Harness + TAP::: Added for CPAN make test support

Impact

  • Moo test pass rate increased from 591 to 687 tests (+96)
  • Moo::Role now correctly exports has, with, requires
  • jcpan -t Moo now runs tests successfully (with minor failures)
  • No more "Error while parsing version number" warnings
  • CPAN.pm no longer tries to install Scalar::Util from CPAN (detects bundled version)
  • All 164 unit tests still pass

Test Plan

  • Unit tests pass (164/164)
  • Moo::Role functions are now exported correctly
  • Moo tests improved from 591 to 687 passing
  • jcpan -t Moo runs and executes test harness

Generated with Devin

fglock and others added 8 commits March 15, 2026 10:38
When a module's import method uses 'goto &other::import', the
TAILCALL marker was being discarded by the parseUseDeclaration
code path. This caused Moo::Role (which does 'goto &Role::Tiny::import')
to fail to export 'has', 'with', 'requires' etc.

Added TAILCALL trampoline loop in StatementParser.parseUseDeclaration()
to handle tail calls in import methods, matching the pattern used in
WarnDie.java and other call sites.

This fix increases Moo test pass rate from 591 to 687 tests (+96).

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The stub Makefile generated by MakeMaker was just echoing a message
for the 'test' target. Now it actually runs all t/*.t test files
using jperl.

This allows 'jcpan -t Module' to properly run module tests.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- MakeMaker now generates MYMETA.yml with prerequisites for CPAN.pm
- Test target uses Test::Harness for cross-platform compatibility
- Added MM_PerlOnJava.pm stub for future full MakeMaker integration
- MM.pm detects PerlOnJava on both Unix and Windows

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Moo::Role exports fixed (has, with, requires)
- Test pass rate: 591 → 687 tests (+96)
- jcpan improvements documented

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Added protected: true for MakeMaker.pm, MM.pm, MM_Unix.pm to prevent
sync.pl from overwriting PerlOnJava-specific implementations.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- VersionHelper: Handle "undef" and non-numeric version strings
  gracefully by returning "0.0.0" instead of throwing NumberFormatException
- MakeMaker: Load ExtUtils::MM to set up MM package so that
  MM->parse_version() works when CPAN.pm needs it

This fixes "Error while parsing version number" warnings and
"For input string: 'undef'" crashes during jcpan operations.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- SubUtil.java: New Java implementation of Sub::Util with set_subname,
  subname, prototype, set_prototype (required by Moo)
- ScalarUtil.java, ListUtil.java: Add $VERSION so CPAN.pm can detect
  bundled versions
- Scalar/Util.pm, List/Util.pm, Sub/Util.pm: XSLoader stub files for
  CPAN.pm version detection
- Test/Harness.pm, TAP/*: Add Test::Harness for CPAN make test support
- config.yaml: Add Test::Harness to sync.pl imports

This enables jcpan -t Moo to run tests (with some test failures
remaining due to regex escaping differences).

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Document known Moo test failures: DEMOLISH, SUPER::new, regex escaping
- Add Phase 10: jcpan Test::Harness integration
- Update success criteria and remaining jcpan improvements
- Add ~85% test pass rate status (687/~800)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
@fglock fglock force-pushed the fix/goto-tailcall-import branch from 2c313e9 to a00404a Compare March 15, 2026 10:35
fglock and others added 21 commits March 15, 2026 12:07
In Perl, `return @array` in scalar context should return the array
count, not the last element. This was causing TAP::Harness panics
because passed() was returning wrong values.

JVM backend: Added emitRuntimeContextConversion() in EmitVariable.java
that checks wantarray register and calls .scalar() for arrays/hashes.

Interpreter backend: Added SCALAR_IF_WANTARRAY opcode (388) that
checks callContext at runtime and converts to scalar if needed.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Phase 11: Fix return @array in scalar context
- Test results: 685/774 subtests (88%), 40/71 test programs
- Updated quotemeta issue: escaping underscores

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Standard Perl doesn't emit these warnings, so it's a PerlOnJava bug.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
1. quotemeta: Don't escape underscore (_) - Perl treats it as part of \w
   - Fixes Moo test failures in t/accessor-coerce.t, t/accessor-isa.t
   - Error messages now show "plus_three" instead of "plus\_three"

2. Package::SUPER::method: Handle explicit package SUPER syntax
   - Moo uses $class->Package::SUPER::new(@_) to call parent constructors
   - Previously only SUPER::method (no package prefix) was supported
   - Now handles Package::SUPER::method by extracting package name and
     looking up method in that package's parent hierarchy

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Phase 12: quotemeta underscore fix (FIXED)
- Phase 13: Package::SUPER::method resolution (FIXED)
- Updated Known Issues to reflect fixed items
- Added next steps for remaining issues

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The emitRuntimeContextConversion() method was leaving different types on
the JVM stack depending on which branch was taken (RuntimeScalar vs
RuntimeArray/Hash), causing VerifyError at class load time.

Fixes:
1. EmitVariable.java: Store branch results to a register and reload after
   merge point, ensuring consistent RuntimeBase type on stack
2. EmitLiteral.java: In RUNTIME context, use generic add(RuntimeBase)
   instead of type-specific add() methods since array/hash elements may
   have been converted to RuntimeBase by emitRuntimeContextConversion()

This fixes Digest::MD5, Digest::SHA, and Getopt::Long tests that were
failing with 'Bad type on operand stack' VerifyError.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Replace mvn/gradlew commands with make/make dev
- Add warning: always commit before switching branches to prevent
  losing uncommitted changes when git stash pop has conflicts

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Add prominent 'ALWAYS use make commands' warning to all skill files
- Add clear table explaining 'make' vs 'make dev' commands
- Make it explicit that 'make dev' skips tests
- Ensure consistency across all 9 skill files and AGENTS.md

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- FileHandle.java: Parse { identifier() } as function call expression, not bareword
- FileHandle.java: Add fallback to parse any bracketed expression as filehandle
- EmitOperator.java: Use register spilling in handleSayOperator to fix ASM frame crash

Fixes 'Odd number of elements in anonymous hash' warnings in TAP::Harness
when using print/say/printf with function call filehandles like:
  print { get_fh() } "text";

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Work was lost during debugging when git stash was used to temporarily
revert changes. Added prominent warnings to AGENTS.md and all SKILL.md
files to prevent this from happening again.

Alternative approaches documented:
- Commit to a WIP branch before testing alternatives
- Use git diff > backup.patch to save uncommitted changes

Also updates moo_support.md with Phase 15 documentation.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Root cause: `print { $self->stdout }` and `print { shift->stdout }` were
being miscompiled as anonymous hashes instead of filehandle blocks.

FileHandle.java fixes:
- When `hasBracket` is true and token is `$`, parse as full expression
  (not just primary) to capture method chains like `$self->stdout`
- When identifier is followed by `->`, parse as expression (for `shift->stdout`)
- Added early detection of hash patterns: `{ identifier => }` or
  `{ identifier , }` returns null immediately without consuming `{`

Result: All "Odd number of elements in anonymous hash" warnings
eliminated from Moo tests.

Test: `print { $self->stdout } @_` now works correctly

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Root cause: local @_ inside string eval was incorrectly throwing
Can t localize lexical variable @_ error.

The issue was that @_ is registered as reserved in the symbol table
but the localization check only excluded our variables.

Fix: Added isReservedVariable() check alongside isOurVariable() check
in all places that validate local variable declarations.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The 'or do {}' construct in Class::Method::Modifiers::install_modifier
doesn't execute correctly when called via Moo's _install_modifier chain.
This causes around on missing methods to create infinite loops instead
of throwing "method not found" errors.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Narrowed down the bug to a specific scenario:
- Bug appears only after use Moo (i.e., after Moo->import)
- Triggered by _install_tracked during Moo's import phase
- Affects or do {} construct in CMM::install_modifier
- Direct calls to CMM always work correctly
- Manually replicating _install_tracked operations doesn't trigger bug
- Appears to be JVM bytecode compilation issue with loaded modules

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The parser now correctly distinguishes between filehandle blocks and
anonymous hashes in more cases:
- { "key", value } - string key followed by comma
- { "key" => value } - string key with fat comma
- { 1, 2 } - numeric keys
- Whitespace between tokens is now properly skipped

This fixes cases where `print { "a", 2 }` was incorrectly parsed as
a filehandle block instead of printing a hash reference.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Added Phase 17 documentation for the print { hash } detection
improvements that handle string keys, numeric keys, and whitespace.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Previously caller() only returned 4 elements (package, filename, line,
subroutine). Carp.pm expects up to 11 elements to work correctly.

Added elements 5-11:
- hasargs: 1 for subroutine calls, undef for eval/main
- wantarray: undef (TODO: track per-frame context)
- evaltext: undef (TODO: capture eval text for string evals)
- is_require: undef (TODO: distinguish require from regular code)
- hints: 0 (compile-time $^H)
- bitmask: undef (compile-time warnings)
- hinthash: undef (compile-time %^H)

Also fixed @db::args population to work outside debug mode, preventing
the "Incomplete caller override detected" message from Carp.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
When accessing a stash entry like $stash->{"xyz"} for a nonexistent key,
RuntimeStash.get() was calling getGlobalCodeRef() which auto-creates an
entry in globalCodeRefs with type=CODE but with an undefined RuntimeCode.

Later, Universal.can() would:
1. Call InheritanceResolver.findMethodInHierarchy() which correctly
   returned null (skipping undefined code refs)
2. Then fall back to checking existsGlobalCodeRef() which returned true
   for the auto-created entry
3. Return the undefined code ref without checking if it was defined

This caused can() to return a value that:
- defined() returned false (correct)
- ref() returned empty (correct)
- Stringification returned CODE(...) (WRONG - should be empty)

This broke Class::Method::Modifiers around() which uses can() to check
if a method exists before wrapping it.

Fixes:
1. Universal.can(): Add getDefinedBoolean() check before returning code ref
2. RuntimeStash.get(): Use new isGlobal*Defined() methods to check for
   existence without auto-creating entries
3. GlobalVariable: Add helper methods that check existence AND defined
   status without auto-creating entries

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
fglock and others added 7 commits March 15, 2026 17:57
When a subroutine is redefined via eval, saved code references (from \&sub
or can()) should continue pointing to the old implementation. This matches
Perl behavior where: my $orig = \&foo; sub foo {new}; $orig->() returns old

Changes:
- SubroutineParser: Create new RuntimeCode when redefining a sub that already
  has code (subroutine, methodHandle, codeObject, or compilerSupplier set)
- RuntimeCode.createCodeReference: Return a snapshot RuntimeScalar instead of
  the global entry, so saved references are not affected by later redefinitions

This fixes the StackOverflowError in Moo around/before/after modifiers,
which rely on can() returning a reference to the original method before
wrapping it.

Moo test results improved from 20/71 to 16/71 failing test programs.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The parser was only checking for 'my' declarations when validating
array arguments to push/unshift. Now also accepts 'our' and 'local'.

This fixes isa-interfere.t which uses: unshift our @isa, 'ExtraClass'

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
When assigning a hash/array reference to a glob (e.g., *INFO = \%Other::INFO),
the previous implementation copied the contents instead of creating an alias.
This broke Moo/Role::Tiny's %INFO aliasing pattern.

Now *foo = \%bar and *foo = \@bar properly create aliases, so both
names refer to the same underlying container.

Fixes compose-roles.t test failures in Moo (4 tests now passing).
Reduces Moo test failures from 15 to 12 test files.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- compose-roles.t now fully passing (25/25 tests)
- Overall Moo tests improved: 58/71 test programs passing (82%)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The List/Util.pm stub was missing the export configuration, so
'use List::Util qw(reduce)' wasn't importing anything.

Added:
- @isa = qw(Exporter)
- @EXPORT_OK with all available functions

Fixes unit/list_util.t test failure in CI.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
The undef operator was incorrectly using RUNTIME context to visit its
operand. This caused emitRuntimeContextConversion() to be applied,
which checks wantarray at runtime. When the containing subroutine was
called in scalar context (e.g., my $r = process()), the hash was
converted to a scalar (key count) before undefine() was called.

This caused undef %hash to silently do nothing, breaking ExifTool's
PDF.pm which relies on clearing module-level %fetched hash between
file processing operations.

The fix changes handleUndefOperator to use LIST context instead of
RUNTIME, ensuring the actual container is passed to undefine().

Also includes fix for isa(main::ClassName) not matching classes
blessed as ClassName - normalizes the argument by stripping main::
or :: prefix before comparison.

Fixes ExifTool PDF.t (26/26 tests now pass, was 7/26)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
@fglock fglock merged commit 604a777 into master Mar 15, 2026
2 checks passed
@fglock fglock deleted the fix/goto-tailcall-import branch March 15, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant