fglock
diff --git a/‎dev/design/NUMERIC_WARNINGS_IMPLEMENTATION.md‎
Lines changed: 188 additions & 0 deletions b/‎dev/design/NUMERIC_WARNINGS_IMPLEMENTATION.md‎
Lines changed: 188 additions & 0 deletions
diff --git a/‎src/main/java/org/perlonjava/core/Configuration.java‎
Lines changed: 1 addition & 1 deletion b/‎src/main/java/org/perlonjava/core/Configuration.java‎
Lines changed: 1 addition & 1 deletion
diff --git a/‎src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java‎
Lines changed: 14 additions & 2 deletions b/‎src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java‎
Lines changed: 14 additions & 2 deletions
diff --git a/‎src/main/java/org/perlonjava/frontend/semantic/ScopedSymbolTable.java‎
Lines changed: 28 additions & 0 deletions b/‎src/main/java/org/perlonjava/frontend/semantic/ScopedSymbolTable.java‎
Lines changed: 28 additions & 0 deletions
diff --git a/‎src/main/java/org/perlonjava/runtime/operators/CompareOperators.java‎
Lines changed: 3 additions & 3 deletions b/‎src/main/java/org/perlonjava/runtime/operators/CompareOperators.java‎
Lines changed: 3 additions & 3 deletions
diff --git a/‎src/main/java/org/perlonjava/runtime/operators/FileTestOperator.java‎
Lines changed: 18 additions & 6 deletions b/‎src/main/java/org/perlonjava/runtime/operators/FileTestOperator.java‎
Lines changed: 18 additions & 6 deletions
@@ -0,0 +1,188 @@
+# Numeric Warnings Implementation Plan
+
+## Problem Statement
+
+Perl's `use warnings "numeric"` should emit warnings like `Argument "abc" isn't numeric` when non-numeric strings are used in numeric context. Currently:
+
+1. `use warnings` and `no warnings` are compile-time pragmas
+2. Numification happens at runtime via `RuntimeScalar.getDouble()` → `NumberParser.parseNumber()`
+3. Runtime code doesn't know the compile-time warning state
+
+### Current Behavior (broken)
+```perl
+use warnings;
+my $x = 0 + "abc";  # Should warn, but doesn't (or warns incorrectly)
+{
+    no warnings "numeric";
+    my $y = 0 + "def";  # Should NOT warn
+}
+my $z = 0 + "ghi";  # Should warn
+```
+
+## Core Design Decision: Warn on Cache Miss
+
+Key insight: `NumberParser` has a numification cache. Most strings are only parsed once.
+We only need to check the warning flag on **cache misses**.
+
+### Flow
+```
+Cache hit path (fast, common):
+  getDouble() → parseNumber() → cache hit → return
+  [no warning check needed]
+
+Cache miss path (rare):
+  getDouble() → parseNumber() → parse string → 
+    if (isNonNumeric && warningsEnabled()) warn → cache result → return
+  [one flag check only on cache miss]
+```
+
+### Behavior Difference from Perl
+
+- **Perl**: Warns every time a non-numeric string is used
+- **Our approach**: Warns only on cache miss (first use of that string)
+
+This is acceptable. We can later add a warning flag to the cache entry if exact Perl behavior is needed.
+
+## Open Question: How to Track Warning State at Runtime
+
+The compile-time symbol table knows if warnings are enabled, but `parseNumber()` runs at runtime and needs to check this. Several options exist with different trade-offs.
+
+### Option A: Perl Global Variable with `local`
+
+Use a Perl global variable `$warnings::_numeric_enabled` that:
+- Is set to 1 by `use warnings "numeric"`
+- Is set to 0 by `no warnings "numeric"` using `local` for automatic scope restore
+
+```java
+public static boolean isNumericWarningsEnabled() {
+    return getGlobalVariable("warnings::_numeric_enabled").getBoolean();
+}
+```
+
+**Pros:**
+- Automatic block scoping via existing `local`/DynamicVariableManager
+- Handles `goto` correctly (DynamicVariableManager already handles this)
+- Simple to implement
+
+**Cons:**
+- Hash lookup on every cache miss (slower than ThreadLocal)
+
+### Option B: ThreadLocal with try/finally
+
+```java
+private static final ThreadLocal<Boolean> numericWarningsEnabled = 
+    ThreadLocal.withInitial(() -> false);
+
+// Compiler generates for "no warnings" blocks:
+boolean _saved = Warnings.isNumericWarningsEnabled();
+Warnings.setNumericWarningsEnabled(false);
+try {
+    // block code
+} finally {
+    Warnings.setNumericWarningsEnabled(_saved);
+}
+```
+
+**Pros:**
+- Fast ThreadLocal read
+- Proper block scoping
+
+**Cons:**
+- **Conflicts with `goto`** - JVM bytecode issues when goto jumps out of try/finally
+- More complex compiler changes
+
+### Option C: Per-Class Static Field + ThreadLocal on Entry
+
+Each generated class (subroutine) has a compile-time constant:
+```java
+public class Sub_foo {
+    static final boolean NUMERIC_WARNINGS = true;  // set at compile time
+}
+```
+
+On subroutine entry, set a ThreadLocal:
+```java
+// Generated at subroutine entry
+Warnings.setNumericWarnings(NUMERIC_WARNINGS);
+```
+
+**Pros:**
+- Fast ThreadLocal read on cache miss
+- No try/finally (no goto issues)
+- Warning state is per-subroutine (matches Perl's lexical scoping)
+
+**Cons:**
+- One ThreadLocal write per subroutine call
+- Nested calls overwrite - need to verify this matches Perl semantics
+- Doesn't handle block-level `no warnings` within a subroutine
+
+### Option D: Simple Global Flag (No Block Scoping)
+
+Just use a simple flag without block-level scoping:
+- `use warnings` → enable globally
+- `no warnings` → disable globally
+
+**Pros:**
+- Simplest implementation
+- No goto issues
+- Correct for 99% of real code (most use file-level `use warnings`)
+
+**Cons:**
+- Block-level `no warnings "numeric"` won't restore on block exit
+- Less correct than Perl
+
+## NumberParser Changes (Common to All Options)
+
+Regardless of which option is chosen for tracking state, the parseNumber() changes are the same:
+
+```java
+// In parseNumber(), after parsing determines the string is non-numeric:
+// (isNonNumeric is already computed during parsing - no extra work)
+if (isNonNumeric && Warnings.isNumericWarningsEnabled()) {
+    WarnDie.warn(new RuntimeScalar("Argument \"" + str + "\" isn't numeric"),
+            RuntimeScalarCache.scalarEmptyString);
+}
+```
+
+Note: `isNonNumeric` is already determined during parsing (e.g., "abc" → 0).
+The only new check is `isNumericWarningsEnabled()`.
+
+## Files to Modify
+
+1. `Warnings.java` - add `isNumericWarningsEnabled()` method (implementation depends on option chosen)
+2. `NumberParser.java` - check flag on cache miss, emit warning
+3. Possibly compiler changes depending on option chosen
+
+## Testing
+
+```perl
+use warnings;
+my $warned = 0;
+local $SIG{__WARN__} = sub { $warned++ };
+
+my $x = 0 + "abc";
+ok($warned == 1, "warns on non-numeric");
+
+$warned = 0;
+my $y = 0 + "123";
+ok($warned == 0, "no warning for numeric string");
+
+$warned = 0;
+{
+    no warnings "numeric";
+    my $z = 0 + "def";
+}
+ok($warned == 0, "no warning in no-warnings block");
+
+# After block, warnings should be restored (if block scoping implemented)
+$warned = 0;
+my $w = 0 + "ghi";
+ok($warned == 1, "warning restored after block");
+```
+
+## Current Status
+
+- [x] Design documented
+- [ ] Decision on runtime state tracking option (A, B, C, or D)
+- [ ] Implementation
+- [ ] Testing with op/numify.t
@@ -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 = "3f00c2f24";
+    public static final String gitCommitId = "c4e439b01";
 
     /**
      * Git commit date of the build (ISO format: YYYY-MM-DD).
 
@@ -165,9 +165,21 @@ static ListNode consumeArgsWithPrototype(Parser parser, String prototype, boolea
 
             // Check for too many arguments without parentheses only if prototype expects 2+ args
             if (!hasParentheses && countPrototypeArgs(prototype) >= 2) {
-                // If we see a comma after parsing all required args, there are too many
+                // If we see a comma after parsing all required args, check if it's a trailing comma
                 if (isComma(TokenUtils.peek(parser))) {
-                    throwTooManyArgumentsError(parser);
+                    // Consume the comma and check what follows
+                    int saveIndex = parser.tokenIndex;
+                    consumeCommas(parser);
+                    LexerToken nextToken = TokenUtils.peek(parser);
+                    // If followed by a statement terminator, it's a trailing comma (allowed)
+                    // Otherwise, it's too many arguments
+                    if (!Parser.isExpressionTerminator(nextToken) && 
+                        nextToken.type != LexerTokenType.EOF &&
+                        !nextToken.text.equals(")")) {
+                        throwTooManyArgumentsError(parser);
+                    }
+                    // Restore position - the comma will be handled by the caller
+                    parser.tokenIndex = saveIndex;
                 }
             }
         }
 
@@ -36,6 +36,8 @@ public class ScopedSymbolTable {
 
     // Stack to manage warning categories for each scope
     public final Stack<BitSet> warningFlagsStack = new Stack<>();
+    // Stack to track explicitly disabled warning categories (for proper $^W interaction)
+    public final Stack<BitSet> warningDisabledStack = new Stack<>();
     // Stack to manage feature categories for each scope
     public final Stack<Integer> featureFlagsStack = new Stack<>();
     // Stack to manage strict options for each scope
@@ -65,6 +67,8 @@ public ScopedSymbolTable() {
             }
         }
         warningFlagsStack.push((BitSet) defaultWarnings.clone());
+        // Initialize the disabled warnings stack (empty by default)
+        warningDisabledStack.push(new BitSet());
         // Initialize the feature categories stack with an empty map for the global scope
         featureFlagsStack.push(0);
         // Initialize the strict options stack with 0 for the global scope
@@ -135,6 +139,8 @@ public int enterScope() {
         inSubroutineBodyStack.push(inSubroutineBodyStack.peek());
         // Push a copy of the current warning categories map onto the stack
         warningFlagsStack.push((BitSet) warningFlagsStack.peek().clone());
+        // Push a copy of the current disabled warnings map onto the stack
+        warningDisabledStack.push((BitSet) warningDisabledStack.peek().clone());
         // Push a copy of the current feature categories map onto the stack
         featureFlagsStack.push(featureFlagsStack.peek());
         // Push a copy of the current strict options onto the stack
@@ -159,6 +165,7 @@ public void exitScope(int scopeIndex) {
             subroutineStack.pop();
             inSubroutineBodyStack.pop();
             warningFlagsStack.pop();
+            warningDisabledStack.pop();
             featureFlagsStack.pop();
             strictOptionsStack.pop();
         }
@@ -528,6 +535,10 @@ public ScopedSymbolTable snapShot() {
         st.warningFlagsStack.pop(); // Remove the initial value pushed by enterScope
         st.warningFlagsStack.push((BitSet) this.warningFlagsStack.peek().clone());
 
+        // Clone disabled warnings flags
+        st.warningDisabledStack.pop(); // Remove the initial value pushed by enterScope
+        st.warningDisabledStack.push((BitSet) this.warningDisabledStack.peek().clone());
+
         // Clone feature flags
         st.featureFlagsStack.pop(); // Remove the initial value pushed by enterScope
         st.featureFlagsStack.push(this.featureFlagsStack.peek());
@@ -631,13 +642,17 @@ public void enableWarningCategory(String category) {
         Integer bitPosition = warningBitPositions.get(category);
         if (bitPosition != null) {
             warningFlagsStack.peek().set(bitPosition);
+            // Clear the disabled bit when enabling
+            warningDisabledStack.peek().clear(bitPosition);
         }
     }
 
     public void disableWarningCategory(String category) {
         Integer bitPosition = warningBitPositions.get(category);
         if (bitPosition != null) {
             warningFlagsStack.peek().clear(bitPosition);
+            // Mark as explicitly disabled (for proper $^W interaction)
+            warningDisabledStack.peek().set(bitPosition);
         }
     }
 
@@ -646,6 +661,15 @@ public boolean isWarningCategoryEnabled(String category) {
         return bitPosition != null && warningFlagsStack.peek().get(bitPosition);
     }
 
+    /**
+     * Checks if a warning category was explicitly disabled via 'no warnings'.
+     * This is used to determine if $^W should be overridden.
+     */
+    public boolean isWarningCategoryDisabled(String category) {
+        Integer bitPosition = warningBitPositions.get(category);
+        return bitPosition != null && warningDisabledStack.peek().get(bitPosition);
+    }
+
     // Methods for managing features using bit positions
     public void enableFeatureCategory(String feature) {
         if (isNoOpFeature(feature)) {
@@ -705,6 +729,10 @@ public void copyFlagsFrom(ScopedSymbolTable source) {
         this.warningFlagsStack.pop();
         this.warningFlagsStack.push((BitSet) source.warningFlagsStack.peek().clone());
 
+        // Copy disabled warnings flags
+        this.warningDisabledStack.pop();
+        this.warningDisabledStack.push((BitSet) source.warningDisabledStack.peek().clone());
+
         // Copy feature flags
         this.featureFlagsStack.pop();
         this.featureFlagsStack.push(source.featureFlagsStack.peek());
 
@@ -128,9 +128,6 @@ public static RuntimeScalar greaterThan(RuntimeScalar arg1, RuntimeScalar arg2)
             return getScalarBoolean((int) arg1.value > (int) arg2.value);
         }
 
-        // Check for uninitialized values
-        checkUninitialized(arg1, arg2, "gt (>)");
-
         // Prepare overload context and check if object is eligible for overloading
         int blessId = blessedId(arg1);
         int blessId2 = blessedId(arg2);
@@ -145,6 +142,9 @@ public static RuntimeScalar greaterThan(RuntimeScalar arg1, RuntimeScalar arg2)
             }
         }
 
+        // Check for uninitialized values (only when using numeric comparison fallback)
+        checkUninitialized(arg1, arg2, "gt (>)");
+
         // Convert strings to numbers if necessary
         arg1 = arg1.getNumber();
         arg2 = arg2.getNumber();
 
@@ -59,10 +59,10 @@ static void updateLastStat(RuntimeScalar arg, boolean ok, int errno, boolean was
         lastStatErrno = errno;
         lastStatWasLstat = wasLstat;
         Stat.lastNativeStatFields = null;
-        if (!ok) {
-            lastBasicAttr = null;
-            lastPosixAttr = null;
-        }
+        // Always reset BasicFileAttributes - they should only be set by statForFileTest
+        // for real filesystem paths. JAR resources don't have BasicFileAttributes.
+        lastBasicAttr = null;
+        lastPosixAttr = null;
     }
 
     static void updateLastStat(RuntimeScalar arg, boolean ok, int errno) {
@@ -128,10 +128,11 @@ private static boolean statForFileTest(RuntimeScalar arg, Path path, boolean lst
             } catch (UnsupportedOperationException | IOException ignored) {
             }
 
-            lastBasicAttr = basicAttr;
-            lastPosixAttr = posixAttr;
             getGlobalVariable("main::!").set(0);
             updateLastStat(arg, true, 0, lstat);
+            // Set attributes after updateLastStat (which resets them to null)
+            lastBasicAttr = basicAttr;
+            lastPosixAttr = posixAttr;
             Stat.lastNativeStatFields = Stat.nativeStat(path.toString(), !lstat);
             return true;
         } catch (NoSuchFileException e) {
@@ -322,6 +323,17 @@ public static RuntimeScalar fileTest(String operator, RuntimeScalar fileHandle)
             }
             // JAR resource path (e.g., "jar:PERL5LIB/DBI.pm")
             if (Jar.exists(filename)) {
+                // Check if it's a directory entry (not a file)
+                if (Jar.isResourceDirectory(filename)) {
+                    updateLastStat(fileHandle, true, 0);
+                    return switch (operator) {
+                        case "-d", "-e", "-r", "-x" -> scalarTrue;  // It's a readable, executable directory
+                        case "-f", "-l", "-w", "-z" -> scalarFalse;  // Not a file, link, writable, or empty
+                        case "-s" -> RuntimeScalarCache.scalarZero;  // Size 0
+                        default -> scalarUndef;
+                    };
+                }
+                // It's a regular file
                 updateLastStat(fileHandle, true, 0);
                 return switch (operator) {
                     case "-e", "-f", "-r" -> scalarTrue;  // Exists, is a file, is readable
Original file line number	Diff line number	Diff line change
`@@ -128,9 +128,6 @@ public static RuntimeScalar greaterThan(RuntimeScalar arg1, RuntimeScalar arg2)`
`128`	`128`	`return getScalarBoolean((int) arg1.value > (int) arg2.value);`
`129`	`129`	`}`
`130`	`130`
`131`		`- // Check for uninitialized values`
`132`		`- checkUninitialized(arg1, arg2, "gt (>)");`
`133`		`-`
`134`	`131`	`// Prepare overload context and check if object is eligible for overloading`
`135`	`132`	`int blessId = blessedId(arg1);`
`136`	`133`	`int blessId2 = blessedId(arg2);`
`@@ -145,6 +142,9 @@ public static RuntimeScalar greaterThan(RuntimeScalar arg1, RuntimeScalar arg2)`
`145`	`142`	`}`
`146`	`143`	`}`
`147`	`144`
	`145`	`+ // Check for uninitialized values (only when using numeric comparison fallback)`
	`146`	`+ checkUninitialized(arg1, arg2, "gt (>)");`
	`147`	`+`
`148`	`148`	`// Convert strings to numbers if necessary`
`149`	`149`	`arg1 = arg1.getNumber();`
`150`	`150`	`arg2 = arg2.getNumber();`