Skip to content

fix: jcpan HTTP::Client::Parallel + op/state.t interpreter regression#593

Merged
fglock merged 2 commits intomasterfrom
fix/interpreter-bareblock-isloop
Apr 28, 2026
Merged

fix: jcpan HTTP::Client::Parallel + op/state.t interpreter regression#593
fglock merged 2 commits intomasterfrom
fix/interpreter-bareblock-isloop

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Apr 28, 2026

Summary

Two related fixes (supersedes #592):

1. jcpan -t HTTP::Client::Parallel (commit 15c0542)

Two unrelated issues blocked the module:

  • POSIX missing exports. POE::Wheel::Run does use POSIX qw(sysconf setsid _SC_OPEN_MAX ECHO ICANON IEXTEN ISIG BRKINT ICRNL INPCK ISTRIP IXON CSIZE PARENB OPOST TCSANOW). The _const_* helpers and POSIX::ECHO etc. were already defined, but the names were not in @EXPORT_OK, so the import died with "ECHO" is not exported by the POSIX module. Added the termios + _SC_* families to @EXPORT_OK and a small sysconf() stub returning sensible defaults (e.g. _SC_OPEN_MAX => 1024).
  • open 0 (1-arg open of a numeric literal). Module::Install::DSL (shipped by LWP::Online) does open 0 or die ...; $dsl = join "", <0>. PerlOnJava crashed with Modification of a read-only value attempted at ./inc/Module/Install/DSL.pm line 14: the 1-arg open path called set() even on readonly literal scalars and read the filename from $_ instead of the global scalar of the same name as the filehandle. Rewrote the 1-arg open block in IOOperator.java to derive the filehandle name from a globref / bareword / numeric literal, look up $main::<name> for the filename (so open 0 reads from $0), register the IO on the named glob, and only write back to args[0] when it is a writable scalar.

2. op/state.t regression (commit 0c40001)

PR #589 (fix(given/when): implicit break and skip smartmatch on boolean exprs) appended an implicit last; to every when block and marked the synthesized given block with BlockNode.isLoop = true so the unlabeled last exits the given block instead of escaping to the enclosing loop.

The JVM backend (EmitBlock.java) honors isLoop. The interpreter backend (BytecodeCompiler.visit(BlockNode)) did not — it never pushed a LoopInfo for a bare BlockNode marked isLoop=true. So when op/state.t falls back to the interpreter ("Method too large, using interpreter backend"), the implicit last from when walked past the given block on loopStack (where it wasn't even pushed) and broke out of the surrounding foreach, turning 4 iterations into 1 and dropping op/state.t from 159/170 to 156/170.

Fix: in BytecodeCompiler.visit(BlockNode), when node.isLoop is true, push a LoopInfo(node.labelName, bodyStartPc, true) before compiling the statements (matches JVM EmitBlock's pushLoopLabels(... isBareBlock, isBareBlock) semantics — bare blocks are unlabeled-control-flow targets) and patch break/next/redo jumps after the body so block teardown (RESTORE_REGEX_STATE, POP_LOCAL_LEVEL, exitScope) still runs on last/next.

Test plan

  • make (full unit-test suite) passes.
  • jcpan -t LWP::Online — PASS (8/8 tests).
  • jcpan -t HTTP::Client::Parallel — PASS (live-internet test correctly skipped).
  • op/state.t recovers from 156/170 → 159/170, with all 4 "given" subtests passing cleanly.

Generated with Devin

@fglock fglock changed the title fix(interpreter): honor BlockNode.isLoop for last/next/redo fix: jcpan HTTP::Client::Parallel + op/state.t interpreter regression Apr 28, 2026
fglock and others added 2 commits April 28, 2026 18:01
Two unrelated issues blocked HTTP::Client::Parallel:

1. POSIX.pm did not export the termios constants nor the _SC_*
   sysconf() name constants used by POE::Wheel::Run (loaded
   transitively from HTTP::Client::Parallel via POE). The Java side
   already implemented _const_ECHO, _const_TCSANOW, etc., and
   POSIX.pm wired them into POSIX::ECHO, POSIX::TCSANOW, ... but
   they were missing from @EXPORT_OK so `use POSIX qw(ECHO ...)` died
   with "ECHO is not exported by the POSIX module". Add the termios
   and _SC_* names to @EXPORT_OK and provide a small sysconf() stub
   that returns sensible defaults (e.g. _SC_OPEN_MAX => 1024).

2. The 1-argument form `open FILEHANDLE` (used by Module::Install::DSL,
   which LWP::Online ships) crashed with "Modification of a read-only
   value" when the filehandle was given as a numeric literal such as
   `open 0`. The runtime tried to call set() on the readonly arg and
   was also picking the filename from $_ instead of from the global
   scalar of the same name as the filehandle. Rewrite the 1-arg open
   path to derive the filehandle name from the value (covering glob
   refs, bareword strings, and numeric literals like 0), look up the
   filename in $main::<name>, and only assign back to args[0] when it
   is a writable scalar.

With both fixes, `jcpan -t HTTP::Client::Parallel` and its
LWP::Online dependency build and pass their unit tests.

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

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The interpreter backend (BytecodeCompiler) ignored `BlockNode.isLoop`,
so synthesized `last` operators inside bare blocks marked as a loop
target (e.g. the implicit break appended to `when` clauses, eval-block
loop control) escaped to the nearest real enclosing loop. The JVM
backend already does the right thing in EmitBlock — make the interpreter
match.

Symptom that triggered this: when state.t falls back to the interpreter
("Method too large, using interpreter backend"), the implicit `last`
emitted by `when (...)` to break out of the surrounding `given` block
was instead breaking out of the `foreach` containing the given. That
turned 4 passing "given" subtests into 1, taking op/state.t from
159/170 to 156/170.

Fix: in `BytecodeCompiler.visit(BlockNode)`, when `node.isLoop` is
true, push a `LoopInfo` (isTrueLoop=true, redo target = body start)
before compiling the statements, and patch the recorded break/next
jumps to the post-body PC and redo jumps to the body start before
popping. Locals are still restored because `last/next` jump to the
PC of `RESTORE_REGEX_STATE` / `POP_LOCAL_LEVEL`, which then run as
normal block teardown.

After this fix, op/state.t is back to 159/170 and the 4 "given"
subtests all pass cleanly.

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

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock fglock force-pushed the fix/interpreter-bareblock-isloop branch from 0c40001 to 4eba31f Compare April 28, 2026 16:02
@fglock fglock merged commit 25b6fa9 into master Apr 28, 2026
2 checks passed
@fglock fglock deleted the fix/interpreter-bareblock-isloop branch April 28, 2026 16:02
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