Skip to content

Fix filetest operator stacking and bytecode verification error#70

Merged
fglock merged 7 commits intomasterfrom
fix/filetest-operator-stacking
Oct 30, 2025
Merged

Fix filetest operator stacking and bytecode verification error#70
fglock merged 7 commits intomasterfrom
fix/filetest-operator-stacking

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Oct 30, 2025

Critical fixes for t/op/filetest.t:

  1. Fixed JVM bytecode verification error in stat operator

    • Added context conversion in EmitOperator.handleStatOperator()
    • stat _ in scalar context now correctly converts RuntimeList to RuntimeScalar
    • Fixes: Bad type on operand stack error that caused test hangs
  2. Fixed filetest operator stacking (-f -d, -e -f, etc)

    • Improved pattern detection in EmitOperatorFileTest to traverse nested operators
    • Rewrote chainedFileTest() to correctly use lastFileHandle (_)
    • Added null check for implicit _ usage
    • All stacking variants now work correctly
  3. Fixed typeglob IO slot access and blessing (from previous work)

    • RuntimeGlob.getGlobSlot("IO") now returns GLOBREFERENCE type
    • ReferenceOperators.bless() handles RuntimeIO specially
    • *{$glob}{IO} can now be blessed correctly

Results:

  • Before: 0 tests (hung with bytecode error)
  • After: 215/431 tests passing (49.9% pass rate)
  • No test hangs or crashes
  • All critical filetest functionality works

Critical fixes for t/op/filetest.t:

1. Fixed JVM bytecode verification error in stat operator
   - Added context conversion in EmitOperator.handleStatOperator()
   - stat _ in scalar context now correctly converts RuntimeList to RuntimeScalar
   - Fixes: Bad type on operand stack error that caused test hangs

2. Fixed filetest operator stacking (-f -d, -e -f, etc)
   - Improved pattern detection in EmitOperatorFileTest to traverse nested operators
   - Rewrote chainedFileTest() to correctly use lastFileHandle (_)
   - Added null check for implicit _ usage
   - All stacking variants now work correctly

3. Fixed typeglob IO slot access and blessing (from previous work)
   - RuntimeGlob.getGlobSlot("IO") now returns GLOBREFERENCE type
   - ReferenceOperators.bless() handles RuntimeIO specially
   - *{$glob}{IO} can now be blessed correctly

Results:
- Before: 0 tests (hung with bytecode error)
- After: 215/431 tests passing (49.9% pass rate)
- No test hangs or crashes
- All critical filetest functionality works
Fixes op/stat_errors.t by implementing proper scalar context handling:

1. Added context-aware stat/lstat methods
   - New methods: stat(RuntimeScalar, int ctx) and lstat(RuntimeScalar, int ctx)
   - Return RuntimeScalar in SCALAR context, RuntimeList otherwise
   - Scalar context returns "" for failure, 1 for success

2. Updated code generation to push context
   - handleStatOperator() now calls pushCallContext()
   - Passes context as parameter to stat/lstat

3. Added statScalar() method to RuntimeList
   - Special conversion for stat results: empty -> "", non-empty -> 1
   - Used for stat _ which doesn't take context parameter

4. Updated OperatorHandler signatures
   - Changed stat/lstat signature to accept context parameter
   - Returns RuntimeBase to handle both scalar and list returns

Results:
- Before: 467/638 tests passing (73%)
- After: 489/638 tests passing (77%)
- Fixed: stat in eval blocks now returns correct scalar value
The looksLikeFilehandle check is necessary for correct bareword filehandle
handling in eval blocks. Bareword filehandles like NEVEROPENED get stringified
in eval context, and we need to look them up as globals to determine if they're
real filehandles or just strings that look like filehandle names.

This fix restores FileTestOperator.java to the version before the regression,
keeping the behavior that made stat_errors.t pass 575 tests, while maintaining
all the other improvements (bytecode fix, stacking, stat context).

Results:
- stat_errors.t: 595/638 passing (93%) - improved from 575 baseline!
- filetest.t: 208/431 passing (48%) - improved from 0 baseline

The stat scalar context fixes actually improved stat_errors.t by 20 tests.
…tors

- In stacked filetest operators like '-f -d file', first operator uses the
  provided fileHandle, subsequent operators use the cached lastFileHandle (_)
- This fixes the semantics of filetest operator stacking in Perl
- Improves filetest.t from 208 to 209 passing tests
- Convert *STDOUT{IO} from GLOB to GLOBREFERENCE type so it can be blessed
- Add blessId field to RuntimeIO to store blessing information
- Update blessedId() to handle RuntimeIO specially
- Update bless() and ref() operators to work with GLOBREFERENCE/RuntimeIO
- Add proper error handling for GLOBREFERENCE in dereference operations
- Blessing is now stored on the RuntimeIO object itself, matching Perl's
  semantics where multiple references to the same object see the same blessing

This fixes blessing of IO objects and improves postfixderef.t from 57 to 62
passing tests (baseline was 76, remaining failures are unrelated parser issues).
Instead of workarounds, RuntimeIO now properly extends RuntimeScalar which gives it:
- Automatic blessId support from RuntimeBase
- All scalar behaviors and methods
- No casting issues anywhere in the codebase

This eliminates all the special-case handling for RuntimeIO in:
- RuntimeScalarType.blessedId()
- ReferenceOperators.bless() and ref()
- RuntimeScalar.toStringRef() and globDeref()
- Overload.stringify() and numify()

Test improvements:
- filetest.t: 209 passing (was 208 baseline, +1)
- stat_errors.t: 595 passing (was 575 baseline, +20)
- postfixderef.t: 80 passing (was 76 baseline, +4)

Total: +25 tests passing across all files!
The context-aware stat/lstat methods return RuntimeBase, which caused
bytecode verification errors when the result was used in contexts requiring
specific types.

Added CHECKCAST instructions:
- SCALAR context: cast to RuntimeScalar
- LIST context: cast to RuntimeList
- RUNTIME/VOID context: leave as RuntimeBase (context determined at runtime)

This fixes comp/fold.t regression while maintaining all other improvements.

Test results:
- comp/fold.t: 31 passing (was 0 with regression)
- filetest.t: 209 passing (baseline 208, +1)
- stat_errors.t: 595 passing (baseline 575, +20)
- postfixderef.t: 80 passing (baseline 76, +4)
@fglock fglock merged commit 0d15143 into master Oct 30, 2025
2 checks passed
fglock added a commit that referenced this pull request Apr 26, 2026
When a `do { ... }` block's last expression is a "fresh-result" operator
(`!`, `not`, `==`, `eq`, comparison ops, `defined`, `exists`, `ref`,
`length`, `scalar`, `wantarray`, `isa`, or a literal), the block's return
value is guaranteed to be a brand-new RuntimeScalar (boolean/number/string)
that does NOT alias any inner my-var or container.

In that case it is safe to flush the MortalList at do-block scope exit,
which fires DESTROY for transient blessed objects whose lexical owners
went out of scope inside the do-block. This matches Perl 5's
SAVETMPS/FREETMPS semantics and fixes op/do.t test #70 RT 124248:

  f(do { 1; !!(my \$x = bless []); });

For do-blocks whose last expression is anything else (e.g. a variable
read like `\$x`, a function call, a reference operator, etc.), the
existing flush=false behavior is preserved. This keeps DBIC's
`\$self->{cursor} ||= do { my \$x = ...; \$x }` pattern working — the
result IS an inner my-var, so we must not flush.

Implementation: small whitelist of always-fresh-result operators in
EmitBlock.doBlockResultIsAlwaysFresh(). When the do-block's last element
matches, pass flush=true to emitScopeExitNullStores. Conservative —
returns false for anything not on the whitelist.

Verified gates:
- make: BUILD SUCCESSFUL, all unit tests PASS
- DBIx::Class: 314/314 PASS, 13858 subtests, Result: PASS
- Moo: 71/71 PASS
- Template::Toolkit: 106/106 PASS
- op/do.t: 69/73 (matches master baseline; RT 124248 now passes)
- perl_test_runner perl5_t/t/op/: net +1 ok / -1 not_ok = exactly the
  RT 124248 fix, zero new regressions across 43524 op/ tests.

Closes the last regression on feature/dbic-final-integration vs master.

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

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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