Fix: goto &sub in use/import not executing target subroutine#320
Merged
Conversation
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>
2c313e9 to
a00404a
Compare
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>
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>
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
When a module's import method uses
goto &other::import, the TAILCALL marker was being discarded by theparseUseDeclarationcode path. This causedMoo::Role(which doesgoto &Role::Tiny::import) to fail to exporthas,with,requiresetc.Root Cause
The
RuntimeCode.apply()call inStatementParser.parseUseDeclaration()was discarding the return value. When the import method returns a TAILCALL marker (fromgoto &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 inWarnDie.javaand other call sites.Additional Fixes (jcpan/CPAN.pm improvements)
"undef"and non-numeric version strings gracefully (fixes "For input string: undef" crashes)ExtUtils::MMsoMM->parse_version()works (fixes "Error while parsing version number" warnings)SubUtil.java) withset_subname,subname,prototype,set_prototype(required by Moo)$VERSIONto Java implementations + XSLoader stub.pmfiles for CPAN.pm version detectionmake testsupportImpact
Moo::Rolenow correctly exportshas,with,requiresjcpan -t Moonow runs tests successfully (with minor failures)Test Plan
jcpan -t Mooruns and executes test harnessGenerated with Devin