From bf00f448b2d060bfab9eedff57947ec0834a3c2f Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Wed, 22 Apr 2026 16:02:27 +0200 Subject: [PATCH 1/2] fix(B,Sub::Name): honor Sub::Name-assigned CV names in B::GV->NAME B::svref_2object($cv)->GV->NAME previously returned "__ANON__" for CVs renamed via Sub::Name::subname or Sub::Util::set_subname, because B.pm required `defined &{$fqn}` to trust the name. Real-Perl XS Sub::Name updates the CV's CvGV/CvNAME_HEK directly and B reads those fields without consulting any stash entry, so the name is honored even when the sub was never installed as a glob. The `defined &{$fqn}` check was originally added to handle stash deletion/clearing (op/stash.t, uni/stash.t), so it cannot be removed outright. Instead, track an explicit `explicitlyRenamed` flag on RuntimeCode that is set by Sub::Name::subname and Sub::Util::set_subname, and have B.pm trust the name whenever that flag is set. - RuntimeCode: add `explicitlyRenamed` field - SubName.java / SubUtil.java: set the flag when renaming - SubName: expose private helper `Sub::Name::_is_renamed($cv)` - B.pm: consult the flag before falling back to the stash check Impact on Sub-Name-0.28 t/exotic_names.t: 1038 more tests now pass (0 -> 1038 pass out of 1560). Remaining 522 failures are unrelated to Sub::Name and stem from stash-aliasing semantics (`*palatable:: = *{"aliased::native::..."}`). No regressions in op/stash.t / uni/stash.t (75/105 before and after). 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 +-- .../runtime/perlmodule/SubName.java | 27 +++++++++++++++++++ .../runtime/perlmodule/SubUtil.java | 3 +++ .../runtime/runtimetypes/RuntimeCode.java | 6 +++++ src/main/perl/lib/B.pm | 10 ++++++- 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index 6a78ddb0d..6736bfe65 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 = "687b74120"; + public static final String gitCommitId = "a97176ff7"; /** * 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 22 2026 14:52:02"; + public static final String buildTimestamp = "Apr 22 2026 16:01:34"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/SubName.java b/src/main/java/org/perlonjava/runtime/perlmodule/SubName.java index 28aaa4993..3cdd9727d 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/SubName.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/SubName.java @@ -2,6 +2,8 @@ import org.perlonjava.runtime.runtimetypes.*; +import static org.perlonjava.runtime.runtimetypes.RuntimeScalarCache.scalarFalse; +import static org.perlonjava.runtime.runtimetypes.RuntimeScalarCache.scalarTrue; import static org.perlonjava.runtime.runtimetypes.RuntimeScalarType.*; /** @@ -35,6 +37,8 @@ public static void initialize() { subName.defineExport("EXPORT_OK", "subname"); try { subName.registerMethod("subname", null); // No prototype to allow flexible args + // Private helper used by B.pm to detect CVs renamed via Sub::Name::subname. + subName.registerMethod("_is_renamed", null); } catch (NoSuchMethodException e) { System.err.println("Warning: Missing Sub::Name method: " + e.getMessage()); } @@ -81,6 +85,29 @@ public static RuntimeList subname(RuntimeArray args, int ctx) { } code.subName = fullName; } + // Mark the CV as explicitly renamed so B::svref_2object()->GV->NAME + // honors the assigned name even when no matching stash entry exists. + code.explicitlyRenamed = true; return codeRef.getList(); } + + /** + * _is_renamed CODEREF + * + * Private helper for B.pm. Returns a true scalar if the given code + * reference has been explicitly renamed via Sub::Name::subname (or + * Sub::Util::set_subname), otherwise an empty/false scalar. Non-CODE + * arguments yield false. + */ + public static RuntimeList _is_renamed(RuntimeArray args, int ctx) { + if (args.size() != 1) { + return scalarFalse.getList(); + } + RuntimeScalar ref = args.get(0); + if (ref.type != CODE) { + return scalarFalse.getList(); + } + RuntimeCode code = (RuntimeCode) ref.value; + return (code.explicitlyRenamed ? scalarTrue : scalarFalse).getList(); + } } diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/SubUtil.java b/src/main/java/org/perlonjava/runtime/perlmodule/SubUtil.java index b549dc81f..c28ecbe42 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/SubUtil.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/SubUtil.java @@ -142,6 +142,9 @@ public static RuntimeList set_subname(RuntimeArray args, int ctx) { } else { code.subName = fullName; } + // Mark the CV as explicitly renamed so B::svref_2object()->GV->NAME + // honors the assigned name even when no matching stash entry exists. + code.explicitlyRenamed = true; return codeRef.getList(); } } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java index f92cc664b..3478b9a31 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java @@ -343,6 +343,12 @@ public static void clearInlineMethodCache() { public boolean isMapGrepBlock = false; // Flag to indicate this code is an eval BLOCK - non-local return should propagate through it public boolean isEvalBlock = false; + // Flag to indicate this CV has been explicitly renamed via Sub::Name::subname + // (or Sub::Util::set_subname). When set, B::svref_2object($cv)->GV->NAME should + // return the assigned name even if the resulting fully-qualified name does not + // correspond to an installed stash entry — matching real-Perl XS Sub::Name + // behaviour, where the CV's CvGV points to a free-floating GV with the name. + public boolean explicitlyRenamed = false; // Depth of active recursive calls to this subroutine, used by the // "Deep recursion on subroutine" warning. Incremented on entry and diff --git a/src/main/perl/lib/B.pm b/src/main/perl/lib/B.pm index 6e6d0b062..4088154e5 100644 --- a/src/main/perl/lib/B.pm +++ b/src/main/perl/lib/B.pm @@ -139,8 +139,16 @@ package B::CV { # Verify the sub still exists in the stash. Stubs whose # stash entry has been deleted/cleared/undefined should be # treated as anonymous (matching Perl 5's GV anonymization). + # Exception: CVs explicitly renamed via Sub::Name::subname + # (or Sub::Util::set_subname) carry a private flag; in + # real Perl their CvGV points to a free-floating GV with + # the assigned name, and NAME should always reflect that. no strict 'refs'; - if (defined &{"$fqn"}) { + my $renamed = 0; + if (exists $Sub::Name::{_is_renamed}) { + $renamed = Sub::Name::_is_renamed($self->{ref}) ? 1 : 0; + } + if ($renamed || defined &{"$fqn"}) { $self->{_pkg_name} = $pkg; $self->{_sub_name} = $name; $self->{_is_anon} = 0; From b2d22f682347a5cdfd0a1cd425246b94fbc92093 Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Wed, 22 Apr 2026 16:36:21 +0200 Subject: [PATCH 2/2] feat(stash): alias storage + name resolution for *Dst:: = *Src:: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes `*Dst:: = *Src::;` actually alias the namespaces at both the storage and lookup levels, matching Perl 5 semantics — while preserving the existing compile-time-qualified reference behaviour (`&Pkg::foo` keeps pointing at its original CvGV, as real Perl does). Before: *Dst:: = *Src::; \%Dst:: == \%Src:: # false (!!!) sub Dst::foo {}; defined &Src::foo # false (!!!) After: *Dst:: = *Src::; \%Dst:: == \%Src:: # true eval 'sub Dst::foo {}'; defined &Src::foo # true *{"Dst::sym"} = sub {}; defined &Src::sym # true ${"Dst::var"} = 42; ${"Src::var"} # 42 # transitive chains also collapse: *Mid:: = *Src::; *Fin:: = *Mid::; \%Fin:: == \%Src:: # true Implementation (see dev/prompts/stash-aliasing-plan.md for the design): 1. GlobalVariable infrastructure - resolveAliasedFqn(fqn): resolves the package prefix of an FQN through stashAliases. Fast path when the alias map is empty (no hashing, no substring) — this is the overwhelmingly common case. - resolvedStashAliasCache: memoised transitive resolution with a 16-hop cycle cap. Invalidated on setStashAlias/clearStashAlias/ resetAllGlobals. Non-aliased packages map to their own String instance so callers can shortcut the substring+concat via reference equality (`resolved == pkg`). 2. Hash storage unification in RuntimeGlob.set(RuntimeGlob) When both glob names end with "::", also put the source's RuntimeStash into globalHashes under the destination key. This makes `\%Dst:: == \%Src::` true and `*Dst::{HASH} == *Src::{HASH}` true. 3. Resolve-with-fallback on read-side accessors getGlobalVariable / getGlobalArray / getGlobalHash / getGlobalCodeRef and their `exists*` siblings first try the alias-resolved FQN; if no value lives there, they fall back to the raw key. This preserves compile-time-qualified references like `\&MCTest::Base::foo` that point at CVs still living at their original FQN, matching real Perl (which keeps each CV's CvGV pinned to the package it was compiled in — the alias only redirects new writes and runtime symbolic refs). 4. Resolve-on-install for sub declarations defineGlobalCodeRef always resolves through the alias so that `*Dst:: = *Src::; sub Dst::foo {}` installs the sub under Src::foo. SubroutineParser applies resolveAliasedFqn to the computed fullName before splitting into code.packageName / code.subName, so caller() and set_subname report the correct target package. 5. SubroutineParser package/name split bugfix placeholder.subName was being set from the raw subName parameter, which may contain "::" (parsing `sub Dst::foo {}` arrives with subName="Dst::foo"). Combined with alias resolution rewriting the full name to "Src::foo", this produced code.subName="Dst::foo" and code.packageName="Src" and caller() reported "Src::Dst::foo". Derive both halves from the resolved fullName instead. Note: resolveAliasedFqn is *not* invoked inside NameNormalizer. That route was too aggressive — it rewrote compile-time-qualified reads like `\&MCTest::Base::foo`, causing a regression in perl5_t/t/mro/method_caching.t where the sub still exists at its original FQN after aliasing. The resolve-on-install + resolve-then- fallback-on-read split avoids this. Known limitation ---------------- GV-level aliasing — where a stash hash key stores a GV whose own name differs from the hash key, e.g.: ${"palatable::"}{"sub"} = ${"palatable::"}{$encoded_sub}; is not supported. PerlOnJava's flat-map storage can't represent "hash key X points to GV named Y" because globs are currently just FQN strings without a separate name field. Tracked as follow-up in the plan doc; it's what's blocking the remaining 392 failures in Sub-Name-0.28 t/exotic_names.t. Testing ------- - New src/test/resources/unit/stash_aliasing.t with 5 subtests (hash identity, sub installation, caller reporting, symbolic refs, chained aliases) — all pass. - make: all unit tests pass. - perl5_t/t/op/stash.t + perl5_t/t/uni/stash.t: 75/105, unchanged. - perl5_t/t/mro/method_caching.t: 31/36, unchanged from master. - perl5_t/t/mro/method_caching_utf8.t: 28/28, unchanged from master. - Sub-Name-0.28 t/exotic_names.t: +130 tests (1038 -> 1168 / 1560), cumulative with the B/Sub::Name fix in the preceding commit. Design doc: dev/prompts/stash-aliasing-plan.md (new) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- dev/prompts/stash-aliasing-plan.md | 179 +++++++++++++++++ .../org/perlonjava/core/Configuration.java | 4 +- .../frontend/parser/SubroutineParser.java | 16 +- .../runtime/runtimetypes/GlobalVariable.java | 188 +++++++++++++++++- .../runtime/runtimetypes/RuntimeGlob.java | 14 ++ src/test/resources/unit/stash_aliasing.t | 78 ++++++++ 6 files changed, 471 insertions(+), 8 deletions(-) create mode 100644 dev/prompts/stash-aliasing-plan.md create mode 100644 src/test/resources/unit/stash_aliasing.t diff --git a/dev/prompts/stash-aliasing-plan.md b/dev/prompts/stash-aliasing-plan.md new file mode 100644 index 000000000..bee56161e --- /dev/null +++ b/dev/prompts/stash-aliasing-plan.md @@ -0,0 +1,179 @@ +# Fix: Stash aliasing (`*Dst:: = *Src::`) + +## Problem + +Perl's typeglob-to-stash assignment makes the two package namespaces share a single underlying hash: + +```perl +*Dst:: = *Src::; +\%Dst:: == \%Src::; # true +sub Dst::x {} # also visible as Src::x +*Dst::{HASH} == *Src::{HASH}; # true +``` + +In PerlOnJava this does not work: the two `%Pkg::` stash-view hashes remain separate objects, and subsequently-installed subs/variables only appear under the namespace they were explicitly qualified with. + +Impact observed: `Sub-Name-0.28 t/exotic_names.t` — 522 failing assertions in the "natively compiled sub" block, which does `*palatable:: = *{"aliased::native::${pkg}::"}` and then compiles `sub palatable::sub`. The freshly-compiled sub ends up with `caller(0)` reporting `palatable::palatable::sub` instead of `aliased::native::${pkg}::${subname}`. + +Other likely downstream breakage: `namespace::clean`, `Package::Stash`, ::ANON stashes, `Moose::Util::MetaRole` patterns, anything that vivifies a stash by glob-aliasing before installing methods. + +## Current Architecture (summary) + +(See detailed report in the agent investigation — also at the end of this doc.) + +- Everything lives in flat, FQN-keyed maps in `GlobalVariable` (`globalVariables`, `globalArrays`, `globalHashes`, `globalCodeRefs`, `globalIORefs`, `globalFormatRefs`). +- `%Pkg::` is a `RuntimeStash` stored under the flat key `"Pkg::"` in `globalHashes`. It is a *view* — `get("bar")` concatenates `"Pkg::" + "bar"` and looks up each kind of symbol in the flat maps. +- `*Dst:: = *Src::` takes an early-return branch in `RuntimeGlob.set(RuntimeGlob)` (lines 372–377) that only calls `setStashAlias("Dst::", "Src::")` and returns. It does not unify storage and it does not rename any flat-map entries. +- `resolveStashAlias` is consulted in only three places: `InheritanceResolver` (method dispatch / AUTOLOAD), `OverloadContext`, and `SubroutineParser` when installing a sub with an *unqualified* name. Every other lookup ignores it. +- `resolveStashHashRedirect` exists for the HASHREFERENCE case (`*Pkg:: = \%Other::`) and is only consulted by `getGlobalIO`. + +## Fix Strategy + +Two complementary changes: + +### 1. Unify the stash-view hash at the `globalHashes` layer + +In `RuntimeGlob.set(RuntimeGlob)`, when both names end with `::`, in addition to `setStashAlias(...)`, make `globalHashes["Dst::"]` point at the same `RuntimeStash` that `globalHashes["Src::"]` already has: + +```java +RuntimeHash srcStash = GlobalVariable.getGlobalHash(value.globName); +GlobalVariable.globalHashes.put(this.globName, srcStash); +``` + +Effect: `\%Dst:: == \%Src::` becomes true. `*Dst::{HASH}` and `*Src::{HASH}` return refs to the same stash. + +Caveat: `getGlobalHash` currently creates a fresh `RuntimeStash(newName)` on first access; when we replace the entry, iterations over `%Dst::` need to surface `Src::` entries. Because `RuntimeStash.get` looks up flat keys using the stash's `namespace` field, the stored `RuntimeStash` will answer with `Src::…` keys — which is exactly what we want. + +Subtle case: if `globalHashes["Dst::"]` already exists with content, we must either merge or blow it away. Perl's behavior is that `*Dst:: = *Src::` *replaces* `%Dst::` with `%Src::`. We should preserve Perl's semantics: after the assignment, the former `%Dst::` storage is gone and every lookup through `Dst::…` hits `Src::…` instead. See (2) below. + +### 2. Make lookups through the aliased namespace hit the target + +Two tactical approaches: + +**2a — Resolve at lookup time (preferred; minimally invasive):** + +Add an alias-resolution helper to `GlobalVariable`: + +```java +public static String resolveAliasedFqn(String fqn) { + if (stashAliases.isEmpty()) return fqn; // fast path + int idx = fqn.lastIndexOf("::"); + if (idx < 0) return fqn; + String pkg = fqn.substring(0, idx + 2); // "Foo::" + String resolved = resolvePackageAliasCached(pkg); // transitive, cached + if (resolved == pkg) return fqn; // identity: no alias + return resolved + fqn.substring(idx + 2); +} +``` + +**Caching strategy (important for hot-path performance).** Every global symbol read/write will go through this helper, so we must avoid repeated work: + +- **Layer 1 — empty-map fast path.** `stashAliases.isEmpty()` short-circuits the entire resolution. This is the common case (no aliases declared in the program). +- **Layer 2 — transitive resolution cache.** A separate `Map resolvedPackageAliases` stores the fully-resolved package name for each alias key — i.e. `"Dst::" -> "Src::"` after walking the chain `Dst -> Mid -> Src`. `resolvePackageAliasCached(pkg)` does `cache.computeIfAbsent(pkg, p -> walkChain(p))` where `walkChain` iterates `stashAliases` up to a hop cap (e.g. 16), detects cycles, and returns the terminal package (or the input if no alias applies). On a cache hit the helper is effectively one `HashMap.get` plus one `String.lastIndexOf("::")` plus one `substring`+concat; no per-lookup chain walking. +- **Cache invalidation.** Every mutation to `stashAliases` (add/remove/clear) must clear `resolvedPackageAliases` wholesale — chain resolutions aren't locally incremental. `setStashAlias`, `clearStashAlias`, and any stash-wipe path call `resolvedPackageAliases.clear()`. This already aligns with the existing `InheritanceResolver.invalidateCache()` / `clearPackageCache()` pattern. +- **Return-identity trick.** When no alias applies, `resolvePackageAliasCached` returns the **same `String` instance** that was passed in, so the caller can check `resolved == pkg` (reference equality) to skip the substring+concat entirely. The cache stores the input string itself for non-aliased packages — one `HashMap.get` for every negative hit. +- **Non-qualified names.** FQNs without `::` (e.g. plain variable names in the main package that arrive pre-normalised, or special variables) hit the `idx < 0` early-return without touching either cache. + +Call the helper from the FQN-keyed accessors: +- `getGlobalVariable(name)` / `existsGlobalVariable` / `removeGlobalVariable` +- `getGlobalArray(name)` / `existsGlobalArray` / `removeGlobalArray` +- `getGlobalHash(name)` *when the key does not end in `::`* (i.e. `%Foo::x` not `%Foo::`) +- `getGlobalCodeRef(name)` / `existsGlobalCodeRef` / `defineGlobalCodeRef` +- `getGlobalIO(name)` — already uses `resolveStashHashRedirect`; replace with the unified helper or layer on top +- `getGlobalFormatRef(name)` if present + +Also call it from `NameNormalizer.normalizeVariableName` for the already-qualified branch (line 159–161), so that e.g. `sub Dst::x {}` normalises to `"Src::x"` before installation. + +Chain handling: already folded into `resolvePackageAliasCached` above; cycles caught by the hop cap. + +**2b — Rewrite at assignment time (alternative):** + +When aliasing `Dst:: → Src::`, walk the flat maps once and copy-or-reassign all entries whose keys start with `"Dst::"` into `"Src::"` keys (if absent; otherwise the Src entry wins, matching Perl). New writes still need to be intercepted because later `$Dst::x = 1;` compiles to a put under `"Dst::x"`. That means we still need (2a) for future writes. So (2b) is strictly more work than (2a) and gains us nothing; do not pursue. + +### 3. Ensure new installations honor the alias + +`NameNormalizer.normalizeVariableName` must apply `resolveAliasedFqn` to qualified names (the `variable.contains("::")` branch). This fixes `sub Foo::x{}` after `*Foo:: = *Src::;` so that the compiled sub is stored under `Src::x`. + +`SubroutineParser.handleNamedSubWithFilter` already calls `resolveStashAlias` on `packageToUse` for the unqualified case (line 925). The `defineGlobalCodeRef(fullName)` path (line 985–986) will be covered automatically if `defineGlobalCodeRef` calls `resolveAliasedFqn` internally. + +### 4. Invalidate caches + +The existing alias path already calls `InheritanceResolver.invalidateCache()` and `GlobalVariable.clearPackageCache()`. Keep them. Audit for other caches keyed on FQN (OverloadContext, method caches) and invalidate. + +## Proposed Implementation Plan + +Break into commits: + +**Commit 1 — infrastructure:** +- Add `GlobalVariable.resolveAliasedFqn(String)` with fixed-point iteration and cycle cap. +- Unit tests for the helper: no alias → identity; single hop; chain; cycle; non-qualified names pass through unchanged; `"Pkg::"` (stash-view key) passes through unchanged. + +**Commit 2 — hash storage unification at assignment:** +- In `RuntimeGlob.set(RuntimeGlob)` stash branch: after `setStashAlias`, also `globalHashes.put(this.globName, getGlobalHash(value.globName))`. +- Unit test: `*Dst:: = *Src::;` then `\%Dst:: == \%Src::` is true. + +**Commit 3 — route lookups through `resolveAliasedFqn`:** +- `getGlobalVariable` / `existsGlobalVariable` / `removeGlobalVariable` +- `getGlobalArray` / `existsGlobalArray` / `removeGlobalArray` +- `getGlobalCodeRef` / `existsGlobalCodeRef` / `defineGlobalCodeRef` +- `getGlobalIO` (replace `resolveStashHashRedirect` with the unified helper — or keep both, document the relationship) +- `NameNormalizer.normalizeVariableName` qualified branch +- Unit tests: `*Dst:: = *Src::;` then: + - `sub Dst::x {}` is callable as `Src::x` + - `$Dst::v = 1;` then `$Src::v == 1` + - `@Dst::a = (1,2);` then `@Src::a == (1,2)` + - caller() inside `sub Dst::x` reports `Src::x` + +**Commit 4 — regression sweep:** +- `make test` — all unit tests +- `perl dev/tools/perl_test_runner.pl perl5_t/t/op/stash.t perl5_t/t/uni/stash.t` — expect improvement or parity (currently 75/105) +- `jcpan -t Sub::Name` — expect `t/exotic_names.t` to approach 1560/1560 +- Sanity-check `namespace::clean` and `Package::Stash` tests if present in `src/test/resources/module` + +**Commit 5 — documentation:** +- Update `docs/ARCHITECTURE.md` or `dev/design/stash-aliasing.md` describing the invariant that "every flat-map lookup resolves stash aliases first". + +## Risks + +1. **Iteration loops.** `resolveAliasedFqn` must terminate on malformed cycles. Use a hop cap. +2. **Hot-path overhead.** Stash-alias resolution runs on every global lookup. Keep `stashAliases` empty-check fast-path cheap (`if (stashAliases.isEmpty()) return fqn;`). +3. **Stash deletion/clearing.** `RuntimeStash.deleteNamespace` and `undefine` use prefix-based removal on the flat maps. After aliasing, `delete $Dst::{x}` should delete `Src::x` — verify this works via the view; if `RuntimeStash.get` already routes through the alias-aware accessors, deletes should Just Work. +4. **`Sub::Name::subname` interplay.** Once stash aliasing works, B.pm's `defined &{$fqn}` check may now succeed in some Sub::Name cases and make the `explicitlyRenamed` flag redundant — but the flag is still needed for the pure Sub::Name-with-nonexistent-package case, so keep it. +5. **Performance cache invalidation.** The existing `clearPackageCache` / `invalidateCache` calls at alias-set time must stay. Any new alias-dependent cache must also hook in. + +## Follow-up / Non-goals + +- `*Dst:: = \%H` (assigning a real hashref, not a stash-glob) already works via `HASHREFERENCE` branch + `resolveStashHashRedirect`; the unified resolution in this plan should subsume that case. +- Unicode package names and exotic-char stash names are orthogonal to aliasing — handled by existing name-normalisation. +- True hierarchical stashes (replacing flat-map storage) is out of scope. We are fixing the view layer only. + +## Progress Tracking + +### Current Status: Plan drafted (2026-04-22) + +### Next Steps +1. Implement Commit 1 (infrastructure + tests). +2. Verify `make` stays green after each commit. +3. After Commit 3, re-run `jcpan -t Sub::Name` and update this doc with numbers. + +### Open Questions +- Should `*Dst:: = *Src::` be a two-way alias (symmetric) or one-way (Dst → Src)? Perl 5 semantics: one-way — after the assignment, writing to `$Src::x` *does* show up in `$Dst::x` because they share the same hash, so it's effectively symmetric at the hash level. But `delete $Src::{}; delete $Dst::{}` leave the other un-deleted only if they no longer share. Need to verify exact Perl 5 semantics with a small test before committing to "symmetric alias" vs "one-way redirect". + +## References + +- Full architecture report: embedded below (from investigation 2026-04-22). +- Related PR: #541 (Sub::Name / B.pm GV->NAME) — independent fix that surfaced this issue. +- Upstream test exposing the bug: `Sub-Name-0.28 t/exotic_names.t` lines 107–124. + +--- + +### Architecture report (verbatim) + +See conversation log 2026-04-22. Key file pointers: +- `src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java` — flat maps, `setStashAlias`/`resolveStashAlias` (168-190), `resolveStashHashRedirect` (692-702), `getGlobalHash` (361-378), `getGlobalCodeRef` (414-456), `defineGlobalCodeRef` (467-474). +- `src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeGlob.java` — `set(RuntimeScalar)` (206-327), `set(RuntimeGlob)` with stash branch (372-377), `getGlobSlot` HASH case (539-560). +- `src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeStash.java` — view; `get` (92-117), `deleteNamespace` (213-241), `undefine` (385-410). +- `src/main/java/org/perlonjava/runtime/runtimetypes/NameNormalizer.java` — `normalizeVariableName` (124-177); does NOT consult `stashAliases` — fix here. +- `src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java` — `handleNamedSubWithFilter` (917-1101); stash alias rewrite at line 925. +- `src/main/java/org/perlonjava/runtime/mro/InheritanceResolver.java` — lines 316, 346 use `resolveStashAlias`. +- `src/main/java/org/perlonjava/runtime/runtimetypes/OverloadContext.java` — lines 133, 426. diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index 6736bfe65..e0766f0ae 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 = "a97176ff7"; + public static final String gitCommitId = "5ccd1c339"; /** * 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 22 2026 16:01:34"; + public static final String buildTimestamp = "Apr 22 2026 18:57:07"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java index a405e1c98..e16819139 100644 --- a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java +++ b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java @@ -983,6 +983,12 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S // - register the subroutine in the namespace String fullName = NameNormalizer.normalizeVariableName(subName, packageToUse); + // Apply stash-alias resolution so `*Dst:: = *Src::; sub Dst::foo {}` + // installs in Src::foo and reports "Src::foo" from caller(). The + // resolution happens here (not in NameNormalizer) to avoid rewriting + // compile-time read references that should keep pointing at their + // original CvGV — only install-site names are resolved. + fullName = GlobalVariable.resolveAliasedFqn(fullName); RuntimeScalar codeRef = GlobalVariable.defineGlobalCodeRef(fullName); InheritanceResolver.invalidateCache(); @@ -1079,7 +1085,14 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S placeholder.attributes = attributes; } // else: preserve existing attributes (e.g., from forward declaration) - placeholder.subName = subName; + + // Split fullName into package/name so subName never contains "::". + // The raw `subName` parameter may include package qualifiers (e.g. parsing + // `sub Dst::foo { }` arrives here with subName="Dst::foo"), and fullName + // may have been rewritten by a stash alias — always derive both halves + // from fullName so caller()/set_subname see a consistent pair. + int lastSep = fullName.lastIndexOf("::"); + placeholder.subName = lastSep >= 0 ? fullName.substring(lastSep + 2) : subName; // Call MODIFY_CODE_ATTRIBUTES if attributes are present // In Perl, this is called at compile time after the sub is defined. @@ -1095,7 +1108,6 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S // Set packageName from the sub's fully-qualified name (CvSTASH equivalent). // For `sub X::foo { }` in package main, packageName should be "X", not "main". - int lastSep = fullName.lastIndexOf("::"); placeholder.packageName = lastSep >= 0 ? fullName.substring(0, lastSep) : parser.ctx.symbolTable.getCurrentPackage(); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java index 985faba87..afbb124df 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java @@ -129,6 +129,7 @@ public static void resetAllGlobals() { globalGlobs.clear(); isSubs.clear(); stashAliases.clear(); + resolvedStashAliasCache.clear(); globAliases.clear(); declaredGlobalVariables.clear(); declaredGlobalArrays.clear(); @@ -169,11 +170,13 @@ public static void setStashAlias(String dstNamespace, String srcNamespace) { String dst = dstNamespace.endsWith("::") ? dstNamespace : dstNamespace + "::"; String src = srcNamespace.endsWith("::") ? srcNamespace : srcNamespace + "::"; stashAliases.put(dst, src); + resolvedStashAliasCache.clear(); } public static void clearStashAlias(String namespace) { String key = namespace.endsWith("::") ? namespace : namespace + "::"; stashAliases.remove(key); + resolvedStashAliasCache.clear(); } public static String resolveStashAlias(String namespace) { @@ -189,6 +192,98 @@ public static String resolveStashAlias(String namespace) { return aliased; } + /** + * Cache of fully-resolved stash aliases with transitive chains collapsed to + * their terminal package. Keys and values both include the trailing "::". + * Invariant: for any `"Pkg::"` with no alias, the cache stores the SAME + * string instance back, so callers can use reference equality to detect a + * non-alias hit without allocating. Cleared whenever {@link #stashAliases} + * is mutated. + */ + private static final Map resolvedStashAliasCache = new HashMap<>(); + + /** Hop cap for cycle detection in {@link #resolvePackageAliasCached}. */ + private static final int STASH_ALIAS_HOP_CAP = 16; + + /** + * Resolves a package name (with trailing "::") to its terminal target, + * following any {@link #setStashAlias} chain to a fixed point. Result is + * cached. Returns the input string (identity-equal) when no alias applies. + */ + private static String resolvePackageAliasCached(String pkgWithColons) { + String cached = resolvedStashAliasCache.get(pkgWithColons); + if (cached != null) { + return cached; + } + String current = pkgWithColons; + for (int hop = 0; hop < STASH_ALIAS_HOP_CAP; hop++) { + String next = stashAliases.get(current); + if (next == null || next.equals(current)) { + break; + } + current = next; + } + // Use identity when no alias applies so callers can fast-path with ==. + String result = current.equals(pkgWithColons) ? pkgWithColons : current; + resolvedStashAliasCache.put(pkgWithColons, result); + return result; + } + + /** + * Resolves a fully-qualified variable/sub name through any stash aliases + * declared via `*Dst:: = *Src::`. FQNs without "::" are returned unchanged; + * FQNs ending in "::" (the stash-view hash itself, e.g. `%Foo::`) are + * returned unchanged — callers working with those should use + * {@link #resolveStashAlias(String)} directly and the unified hash storage. + * + *

Fast path: if no aliases have been declared, returns the input + * reference unchanged (no hashing, no substring). Hot-path accessors may + * therefore call this unconditionally. + * + * @param fqn a name like "Foo::bar" — may or may not contain "::" + * @return the alias-resolved FQN, or the original reference if unchanged + */ + public static String resolveAliasedFqn(String fqn) { + if (stashAliases.isEmpty() || fqn == null) { + return fqn; + } + int idx = fqn.lastIndexOf("::"); + if (idx < 0) { + return fqn; + } + // "Pkg::" — the stash-view hash itself. Leave it alone. + if (idx == fqn.length() - 2) { + return fqn; + } + String pkg = fqn.substring(0, idx + 2); + String resolved = resolvePackageAliasCached(pkg); + if (resolved == pkg) { // identity: no alias applied + return fqn; + } + return resolved + fqn.substring(idx + 2); + } + + /** + * Migrates all IO entries keyed under {@code dstNs} (a package name ending + * in "::") into the corresponding slots under {@code srcNs}. Called when + * `*Dst:: = *Src::` fires so a DATA filehandle placeholder set up by the + * parser at Dst::DATA remains reachable after lookups start resolving to + * Src::DATA. Entries already present under {@code srcNs} are preserved. + * Scoped to IO only — CVs, scalars, arrays, hashes keep their original + * package binding as in real Perl. + */ + public static void migrateStashIOEntries(String dstNs, String srcNs) { + if (dstNs.equals(srcNs)) return; + int dstLen = dstNs.length(); + for (String key : new java.util.ArrayList<>(globalIORefs.keySet())) { + if (key.startsWith(dstNs) && key.length() > dstLen) { + String newKey = srcNs + key.substring(dstLen); + RuntimeGlob value = globalIORefs.remove(key); + if (value != null) globalIORefs.putIfAbsent(newKey, value); + } + } + } + /** * Sets a glob alias. After `*a = *b`, calling setGlobAlias("a", "b") makes * all slot assignments to "a" also affect "b" and vice versa. @@ -243,6 +338,19 @@ public static java.util.List getGlobAliasGroup(String globName) { * @return The RuntimeScalar representing the global variable. */ public static RuntimeScalar getGlobalVariable(String key) { + // Stash alias resolution with fallback: if the aliased destination has + // a value, use it; otherwise fall through to the raw key. See + // getGlobalCodeRef for the rationale (preserve compile-time-qualified + // refs while letting runtime symbolic refs follow the alias). + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key) { + RuntimeScalar resolved = globalVariables.get(resolvedKey); + if (resolved != null) { + return resolved; + } + } + } RuntimeScalar var = globalVariables.get(key); if (var == null) { // Need to initialize global variable @@ -291,6 +399,12 @@ public static void setGlobalVariable(String key, String value) { * @return True if the global variable exists, false otherwise. */ public static boolean existsGlobalVariable(String key) { + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key && globalVariables.containsKey(resolvedKey)) { + return true; + } + } return globalVariables.containsKey(key) || key.endsWith("::a") // $a, $b always exist || key.endsWith("::b"); @@ -324,6 +438,15 @@ public static RuntimeScalar removeGlobalVariable(String key) { * @return The RuntimeArray representing the global array. */ public static RuntimeArray getGlobalArray(String key) { + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key) { + RuntimeArray resolved = globalArrays.get(resolvedKey); + if (resolved != null) { + return resolved; + } + } + } RuntimeArray var = globalArrays.get(key); if (var == null) { var = new RuntimeArray(); @@ -339,6 +462,12 @@ public static RuntimeArray getGlobalArray(String key) { * @return True if the global array exists, false otherwise. */ public static boolean existsGlobalArray(String key) { + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key && globalArrays.containsKey(resolvedKey)) { + return true; + } + } return globalArrays.containsKey(key); } @@ -365,6 +494,18 @@ public static RuntimeHash getGlobalHash(String key) { if (key.length() > 6 && key.endsWith("::") && key.startsWith("main::")) { key = key.substring(6); } + // Stash alias resolution with fallback for non-stash-view hashes + // (e.g. %Pkg::h). Stash-view keys (ending in "::") are already + // unified at assignment time in RuntimeGlob.set(RuntimeGlob). + if (!stashAliases.isEmpty() && !key.endsWith("::")) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key) { + RuntimeHash resolved = globalHashes.get(resolvedKey); + if (resolved != null) { + return resolved; + } + } + } RuntimeHash var = globalHashes.get(key); if (var == null) { // Check if this is a package stash (ends with ::) @@ -415,6 +556,27 @@ public static RuntimeScalar getGlobalCodeRef(String key) { if (key == null) { return new RuntimeScalar(); } + // Stash aliasing: after `*Dst:: = *Src::`, look up under the resolved + // target first. If no sub exists there, fall back to the raw key so + // that compile-time-qualified references like `\&Pkg::foo` keep + // working when the sub still lives at its original FQN. + // This matches real Perl, which keeps each CV's CvGV pinned to the + // package where it was compiled — the stash alias only redirects + // new writes and runtime hash-view lookups. + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key) { + RuntimeScalar resolvedPinned = pinnedCodeRefs.get(resolvedKey); + if (resolvedPinned != null) { + return resolvedPinned; + } + RuntimeScalar resolvedVar = globalCodeRefs.get(resolvedKey); + if (resolvedVar != null) { + pinnedCodeRefs.put(resolvedKey, resolvedVar); + return resolvedVar; + } + } + } // First check if we have a pinned reference that survives stash deletion RuntimeScalar pinned = pinnedCodeRefs.get(key); if (pinned != null) { @@ -465,10 +627,13 @@ public static RuntimeScalar getGlobalCodeRef(String key) { * @return The RuntimeScalar representing the global code reference. */ public static RuntimeScalar defineGlobalCodeRef(String key) { - RuntimeScalar ref = getGlobalCodeRef(key); + // For defines, always resolve through stash aliases: `*Dst:: = *Src::` + // followed by `sub Dst::foo {}` should install the sub in Src::foo. + String resolvedKey = resolveAliasedFqn(key); + RuntimeScalar ref = getGlobalCodeRef(resolvedKey); // Ensure it's in globalCodeRefs so method resolution finds it - if (!globalCodeRefs.containsKey(key)) { - globalCodeRefs.put(key, ref); + if (!globalCodeRefs.containsKey(resolvedKey)) { + globalCodeRefs.put(resolvedKey, ref); } return ref; } @@ -480,6 +645,12 @@ public static RuntimeScalar defineGlobalCodeRef(String key) { * @return True if the global code reference exists, false otherwise. */ public static boolean existsGlobalCodeRef(String key) { + if (!stashAliases.isEmpty()) { + String resolvedKey = resolveAliasedFqn(key); + if (resolvedKey != key && globalCodeRefs.containsKey(resolvedKey)) { + return true; + } + } return globalCodeRefs.containsKey(key); } @@ -747,7 +918,16 @@ public static RuntimeScalar getGlobalIOCopy(String key) { * @return True if the global IO reference exists, false otherwise. */ public static boolean existsGlobalIO(String key) { - return globalIORefs.containsKey(key); + if (globalIORefs.containsKey(key)) return true; + // Follow stash hash redirects so a bareword-handle existence probe in + // an aliased package sees IO placeholders that got redirected at + // auto-viv time. Without this, the parser path that tries + // parseBarewordHandle("DATA") in package Dst after `*Dst:: = *Src::` + // misses the handle and falls back to the hard-coded main::DATA path, + // which then reads from an empty placeholder. + String resolved = resolveStashHashRedirect(key); + if (!resolved.equals(key) && globalIORefs.containsKey(resolved)) return true; + return false; } /** diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeGlob.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeGlob.java index ed7fb68d8..42e3e58d3 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeGlob.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeGlob.java @@ -371,6 +371,20 @@ public RuntimeScalar set(RuntimeGlob value) { if (this.globName.endsWith("::") && value.globName.endsWith("::")) { GlobalVariable.setStashAlias(this.globName, value.globName); + // Unify the stash-view hash so `\%Dst:: == \%Src::` and `*Dst::{HASH} == *Src::{HASH}`. + // Without this, the two RuntimeStash objects remain distinct even though name-level + // lookups resolve through stashAliases. Perl 5 semantics make the two package hashes + // share the same underlying SV. + RuntimeHash srcStash = GlobalVariable.getGlobalHash(value.globName); + GlobalVariable.globalHashes.put(this.globName, srcStash); + // Migrate any pre-existing IO entries from Dst:: to Src::. Unlike code + // and variable slots (where real Perl keeps the CV/SV pinned to its + // compile-time package), IO handles are usually transient and the + // parser's DATA filehandle placeholder is the common case that + // NEEDS to follow the alias — otherwise `` in the aliased + // package reads an empty handle because the placeholder was set up + // at Dst::DATA before this alias declaration ran. + GlobalVariable.migrateStashIOEntries(this.globName, value.globName); InheritanceResolver.invalidateCache(); GlobalVariable.clearPackageCache(); return value.scalar(); diff --git a/src/test/resources/unit/stash_aliasing.t b/src/test/resources/unit/stash_aliasing.t new file mode 100644 index 000000000..1b9fd5584 --- /dev/null +++ b/src/test/resources/unit/stash_aliasing.t @@ -0,0 +1,78 @@ +#!/usr/bin/perl +use strict; +use warnings; +use Test::More; +no strict 'refs'; + +# Stash aliasing: *Dst:: = *Src:: +# See dev/prompts/stash-aliasing-plan.md + +subtest 'hash identity' => sub { + package StashAliasSrc1; + sub existing_sub { 42 } + package main; + + *StashAliasDst1:: = *StashAliasSrc1::; + ok( \%StashAliasDst1:: == \%StashAliasSrc1::, + '\%Dst:: == \%Src::' ); + is( [keys %StashAliasDst1::]->[0], 'existing_sub', + 'keys %Dst:: shows existing Src entry' ); +}; + +subtest 'sub installed via aliased package is visible under source' => sub { + package StashAliasSrc2; + package main; + *StashAliasDst2:: = *StashAliasSrc2::; + + eval q{ sub StashAliasDst2::runtime_sub { "hello" } }; + die $@ if $@; + + ok( defined &StashAliasSrc2::runtime_sub, 'defined &Src::runtime_sub' ); + is( StashAliasSrc2::runtime_sub(), 'hello', 'call through Src:: works' ); +}; + +subtest 'caller() reports the resolved package name' => sub { + package StashAliasSrc3; + package main; + *StashAliasDst3:: = *StashAliasSrc3::; + + eval q{ sub StashAliasDst3::caller_sub { (caller(0))[3] } }; + die $@ if $@; + + is( StashAliasSrc3::caller_sub(), 'StashAliasSrc3::caller_sub', + 'caller(0)[3] reports the alias target, not Dst::Dst::name' ); +}; + +subtest 'symbolic refs route through the alias' => sub { + package StashAliasSrc4; + our $x = 123; + package main; + *StashAliasDst4:: = *StashAliasSrc4::; + + is( ${"StashAliasDst4::x"}, 123, + 'symbolic $Dst::x sees $Src::x' ); + + *{"StashAliasDst4::runtime_sym"} = sub { "via-sym" }; + ok( defined &StashAliasSrc4::runtime_sym, + 'runtime-installed sub via symbolic Dst:: ref is visible as Src::' ); +}; + +subtest 'chain alias' => sub { + package StashAliasChainSrc; + sub chain_fn { "chain" } + package main; + + *StashAliasChainMid:: = *StashAliasChainSrc::; + *StashAliasChainDst:: = *StashAliasChainMid::; + + ok( \%StashAliasChainDst:: == \%StashAliasChainSrc::, + 'transitive alias: Dst == Src' ); + + eval q{ sub StashAliasChainDst::new_fn { "new" } }; + die $@ if $@; + + ok( defined &StashAliasChainSrc::new_fn, + 'chained alias resolves through to terminal package' ); +}; + +done_testing;