From 78149b0ad7d32e4113acabafb21d4f371e60163a Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Tue, 28 Apr 2026 13:11:57 +0200 Subject: [PATCH] fix: unblock GitLab::API::v4 (and Const::Fast) under jcpan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating `jcpan -t GitLab::API::v4` surfaced four independent defects. Each one was masking the next, so they all need to land together for the module to load and its unit tests to pass. 1. Parser: `use Foo qw()` must skip import (StatementParser.java) Real Perl treats both `use Foo ()` and `use Foo qw()` as the "skip import" form. PerlOnJava only special-cased `()` (parens), so `use Carp qw();` ran `Carp->import()` with no args, which Exporter interprets as "import @EXPORT defaults" and leaks `Carp::croak` into the caller. That made `sub croak { ... }` in GitLab::API::v4::RESTClient look like a redefinition. Fix: also recognise a syntactically empty list expression after the module name (e.g. `qw()`, `((), qw())`) by walking the parsed AST for ListNode emptiness, gated on the parser actually consuming tokens. `use Foo @runtime_empty_list` still calls import(), as in real Perl. 2. HTTP::Tiny: stop pretending to export `new`/`get`/`post`/`request` (HttpTiny.java) Real `HTTP::Tiny` exports nothing — these are object methods. PerlOnJava's HTTP::Tiny added them to @EXPORT, so any package that did `use HTTP::Tiny;` got `Foo::new` aliased to `HTTP::Tiny::new`. Moo's assert_constructor then saw a pre-existing `new` and bailed with "Unknown constructor for Foo already exists" before installing its own constructor, breaking any Moo class that mentioned HTTP::Tiny. Fix: drop the bogus `defineExport("EXPORT", ...)`. (This bug was pre-existing; defect #1 made compilation fail earlier and hid it.) 3. Read-only error message format (RuntimeScalarReadOnly.java) `RuntimeScalarReadOnly.vivify()` threw a plain `RuntimeException`, which makes stringifyException() fall through to the multi-line stack-trace formatter: Modification of a read-only value attempted main at FILE line N main at FILE line N Real Perl, and Test::Fatal-style regex matchers in Const::Fast's t/10-basics.t, expect the canonical single-line form: Modification of a read-only value attempted at FILE line N. Fix: throw `PerlCompilerException` instead so the short-circuit path in stringifyException() runs and the location is appended from PerlCompilerException's buildErrorMessage(). 4. Internals::hv_clear_placeholders no-op (Internals.java) Const::Fast calls `&Internals::hv_clear_placeholders(\%h)` when sealing a hash readonly. PerlOnJava doesn't model Perl's restricted-hash placeholder slots, so there's nothing to clear, but failing with "Undefined subroutine" aborts every call site. Fix: register it as a no-op stub. Verification - `make` (full unit-test suite): green. - `./jcpan -t GitLab::API::v4` now reaches and passes the module's own t/unit.t (was: fatal compile error). - Const::Fast t/10-basics.t goes from 5/15 passing (compilation bailed early on hv_clear_placeholders) to 17/25 passing. The remaining 8 failures are deeper readonly-hash and reassignment semantics that are out of scope here. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../org/perlonjava/core/Configuration.java | 4 +- .../frontend/parser/StatementParser.java | 37 ++++++++++++++++++- .../runtime/perlmodule/HttpTiny.java | 8 +++- .../runtime/perlmodule/Internals.java | 20 ++++++++++ .../runtimetypes/RuntimeScalarReadOnly.java | 8 +++- 5 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index d3357ff1d..251f09a08 100644 --- a/src/main/java/org/perlonjava/core/Configuration.java +++ b/src/main/java/org/perlonjava/core/Configuration.java @@ -33,7 +33,7 @@ public final class Configuration { * Automatically populated by Gradle/Maven during build. * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String gitCommitId = "7d7723dfb"; + public static final String gitCommitId = "e3750ee00"; /** * Git commit date of the build (ISO format: YYYY-MM-DD). @@ -48,7 +48,7 @@ public final class Configuration { * Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at" * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String buildTimestamp = "Apr 28 2026 12:16:39"; + public static final String buildTimestamp = "Apr 28 2026 13:11:57"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/frontend/parser/StatementParser.java b/src/main/java/org/perlonjava/frontend/parser/StatementParser.java index d4587d620..ddad034b0 100644 --- a/src/main/java/org/perlonjava/frontend/parser/StatementParser.java +++ b/src/main/java/org/perlonjava/frontend/parser/StatementParser.java @@ -564,6 +564,29 @@ public static Node parseGivenStatement(Parser parser) { return givenBlock; } + /** + * Returns true if the given AST node is a syntactically empty list — that is, + * a ListNode whose elements are themselves all syntactically empty lists + * (or which has no elements at all). Examples that match: `()`, `qw()`, + * `((), qw())`. Examples that do not match: `@list`, `(1)`, `qw(a)`. + * + * Used by `use` parsing to detect `use Foo qw()` and similar forms, which + * Perl treats as "skip import" — distinct from `use Foo` (no list at all, + * imports defaults) and `use Foo @empty` (calls import even if @empty is + * runtime-empty). + */ + private static boolean isStaticallyEmptyList(Node node) { + if (!(node instanceof ListNode listNode)) { + return false; + } + for (Node child : listNode.elements) { + if (!isStaticallyEmptyList(child)) { + return false; + } + } + return true; + } + /** * Parses a use or no declaration. * @@ -681,7 +704,19 @@ public static Node parseUseDeclaration(Parser parser, LexerToken token) { // Parse the parameter list boolean hasParentheses = TokenUtils.peek(parser).text.equals("("); + int listStartIndex = parser.tokenIndex; Node list = ListParser.parseZeroOrMoreList(parser, 0, false, false, false, false); + // Detect a syntactically empty list expression after the module name + // (e.g. `use Foo qw()` — `use Foo ()` is already covered by hasParentheses). + // Perl treats this as "skip import", distinct from `use Foo` (no list at all) + // which calls import() with no arguments and triggers default exports. + // We require both: (a) the parser actually consumed tokens for a list + // expression (so this isn't `use Foo;`) and (b) the resulting AST is + // statically empty (so this isn't `use Foo @list` where @list happens + // to be empty at runtime — real Perl still calls import() in that case). + boolean hasEmptyLiteralList = !hasParentheses + && parser.tokenIndex > listStartIndex + && isStaticallyEmptyList(list); if (CompilerOptions.DEBUG_ENABLED) ctx.logDebug("Use statement list hasParentheses:" + hasParentheses + " ast:" + list); StatementResolver.parseStatementTerminator(parser); @@ -772,7 +807,7 @@ public static Node parseUseDeclaration(Parser parser, LexerToken token) { RuntimeList args = runSpecialBlock(parser, "BEGIN", list, RuntimeContextType.LIST); if (CompilerOptions.DEBUG_ENABLED) ctx.logDebug("Use statement list: " + args); - if (hasParentheses && args.isEmpty()) { + if ((hasParentheses || hasEmptyLiteralList) && args.isEmpty()) { // do not import } else { // fetch the method using `can` operator diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/HttpTiny.java b/src/main/java/org/perlonjava/runtime/perlmodule/HttpTiny.java index f5fdbaf34..ddb6a5ca1 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/HttpTiny.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/HttpTiny.java @@ -34,7 +34,13 @@ public HttpTiny() { public static void initialize() { HttpTiny httpTiny = new HttpTiny(); httpTiny.initializeExporter(); - httpTiny.defineExport("EXPORT", "new", "get", "post", "request"); + // HTTP::Tiny does not export anything in real Perl. `new`, `get`, `post`, + // and `request` are object methods, not importable subs. Adding them to + // @EXPORT polluted callers (`use HTTP::Tiny;` in `package Foo;` would + // alias `Foo::new` to `HTTP::Tiny::new`), which broke any Moo-based + // class that did `use HTTP::Tiny;` — Moo's assert_constructor saw a + // pre-existing `new` and bailed with "Unknown constructor for Foo + // already exists" before it could install its own constructor. try { httpTiny.registerMethod("request", null); httpTiny.registerMethod("mirror", null); diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java b/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java index ff9d7e64c..06feb32c0 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java @@ -24,6 +24,12 @@ public static void initialize() { try { internals.registerMethod("SvREADONLY", "svReadonly", "\\[$@%];$"); internals.registerMethod("SvREFCNT", "svRefcount", "$;$"); + // Clear placeholder slots in a restricted hash. PerlOnJava doesn't + // implement Perl's restricted-hash placeholder machinery (used by + // Hash::Util / fields), so this is a safe no-op. Modules like + // Const::Fast call it when sealing a hash readonly; failing here + // breaks loading the whole module. + internals.registerMethod("hv_clear_placeholders", "hvClearPlaceholders", "\\%"); // Phase 0 diagnostic: expose PerlOnJava-internal refcount state // (refCount, flags, tracking mode) for differential testing // against native Perl. See dev/design/refcount_alignment_plan.md. @@ -72,6 +78,20 @@ public static RuntimeList stack_refcounted(RuntimeArray args, int ctx) { return new RuntimeScalar(1).getList(); } + /** + * Clear placeholder slots in a restricted hash. + * + * PerlOnJava does not implement Perl's restricted-hash placeholder + * machinery (used by {@code Hash::Util} / pseudo-hashes / fields). The + * actual op only matters for hashes that have had keys "locked" via + * {@code lock_keys}, where calling {@code keys %h} can leave behind + * placeholder slots. We don't have those slots, so there is nothing + * to do — returning an empty list matches the behavior callers expect. + */ + public static RuntimeList hvClearPlaceholders(RuntimeArray args, int ctx) { + return new RuntimeList(); + } + /** * No-op, returns false. * diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalarReadOnly.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalarReadOnly.java index fb3180f97..c99ef470e 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalarReadOnly.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalarReadOnly.java @@ -114,7 +114,13 @@ public RuntimeScalarReadOnly(double d) { */ @Override void vivify() { - throw new RuntimeException("Modification of a read-only value attempted"); + // Use PerlCompilerException so stringifyException() short-circuits and + // appends "at FILE line N." to the message. Throwing a plain + // RuntimeException produces a multi-line stack-trace style message + // (e.g. for `\do{45}; $$r = 99`) that doesn't match Perl's canonical + // "Modification of a read-only value attempted at FILE line N." form + // and breaks Const::Fast / Test::Fatal-style regex matches. + throw new PerlCompilerException("Modification of a read-only value attempted"); } /**