Skip to content

Commit 4bf86e9

Browse files
fglockdevin-ai-integration[bot]
fglock
and
devin-ai-integration[bot]
committed
Implement parse-time strict vars checking (perl #49472)
Add strict vars checking at parse time so that undeclared variables inside lazily-compiled named subroutine bodies are caught at compile time rather than at call time (or never). Key changes: - Variable.java: Add checkStrictVarsAtParseTime() with comprehensive exemption logic mirroring EmitVariable/BytecodeCompiler checks - OperatorParser.java: Set parsingDeclaration flag to suppress strict check while parsing my/our/state variable declarations - Parser.java: Add parsingDeclaration flag - SignatureParser.java: Register signature parameters in the symbol table during parsing so default values and sub body can reference them - SubroutineParser.java: Enter scope for signature variables before parsing signature, exit after block body - StatementParser.java: Register catch variable in scope and suppress strict check for catch parameter declaration Test results: - attrs.t: 158/159 (was 157/159) - test 88 now passes - uni/attrs.t: 35/35 (was 34/35) - test 24 now passes - All unit tests pass Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent af21f2f commit 4bf86e9

File tree

7 files changed

+175
-13
lines changed

7 files changed

+175
-13
lines changed

‎src/main/java/org/perlonjava/core/Configuration.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class Configuration {
3333
* Automatically populated by Gradle/Maven during build.
3434
* DO NOT EDIT MANUALLY - this value is replaced at build time.
3535
*/
36-
public static final String gitCommitId = "4ddc0373d";
36+
public static final String gitCommitId = "af21f2fde";
3737

3838
/**
3939
* Git commit date of the build (ISO format: YYYY-MM-DD).

‎src/main/java/org/perlonjava/frontend/parser/OperatorParser.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,11 @@ static OperatorNode parseVariableDeclaration(Parser parser, String operator, int
315315
}
316316

317317
// Create OperatorNode ($, @, %), ListNode (includes undef), SubroutineNode
318+
// Suppress strict vars check while parsing the variable being declared
319+
boolean savedParsingDeclaration = parser.parsingDeclaration;
320+
parser.parsingDeclaration = true;
318321
Node operand = ParsePrimary.parsePrimary(parser);
322+
parser.parsingDeclaration = savedParsingDeclaration;
319323
if (CompilerOptions.DEBUG_ENABLED) parser.ctx.logDebug("parseVariableDeclaration " + operator + ": " + operand + " (ref=" + isDeclaredReference + ")");
320324

321325
// Add variables to the scope

‎src/main/java/org/perlonjava/frontend/parser/Parser.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ public class Parser {
3838
// Flags to indicate special parsing states.
3939
public boolean parsingForLoopVariable = false;
4040
public boolean parsingTakeReference = false;
41+
// Are we currently parsing a my/our/state declaration's variable list?
42+
// Used to suppress strict vars checking for the variable being declared.
43+
public boolean parsingDeclaration = false;
4144
// Are we parsing the top level script?
4245
public boolean isTopLevelScript = false;
4346
// Are we parsing inside a class block?

‎src/main/java/org/perlonjava/frontend/parser/SignatureParser.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ private void parseParameter() {
164164
paramName = consumeToken().text;
165165
}
166166

167+
// Register the parameter variable in the symbol table immediately so that
168+
// (1) default value expressions for later parameters can reference it, and
169+
// (2) the parse-time strict vars check can find it in the sub body.
170+
// The scope was entered by SubroutineParser before calling parseSignature().
171+
if (paramName != null) {
172+
parser.ctx.symbolTable.addVariable(sigil + paramName, "my", null);
173+
}
174+
167175
if (paramName != null && paramName.equals("_")) {
168176
parser.throwError(paramStartIndex, "Can't use global " + sigil + "_ in subroutine signature");
169177
}

‎src/main/java/org/perlonjava/frontend/parser/StatementParser.java‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -344,28 +344,50 @@ public static Node parseTryStatement(Parser parser) {
344344
// Parse the catch block
345345
TokenUtils.consume(parser, LexerTokenType.IDENTIFIER); // "catch"
346346
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "(");
347+
// Suppress strict vars check for the catch variable — catch ($e) implicitly
348+
// declares $e as a lexical variable, similar to my $e.
349+
boolean savedParsingDeclaration = parser.parsingDeclaration;
350+
parser.parsingDeclaration = true;
347351
Node catchParameter = parser.parseExpression(0); // Parse the exception variable
352+
parser.parsingDeclaration = savedParsingDeclaration;
348353
TokenUtils.consume(parser, LexerTokenType.OPERATOR, ")");
349-
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "{");
350-
Node catchBlock = ParseBlock.parseBlock(parser);
351-
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "}");
352354

353-
// Parse the optional finally block
354-
Node finallyBlock = null;
355-
if (TokenUtils.peek(parser).text.equals("finally")) {
356-
TokenUtils.consume(parser, LexerTokenType.IDENTIFIER); // "finally"
355+
// Register the catch variable in a scope so the parse-time strict vars
356+
// check can find it inside the catch block body.
357+
int catchScopeIndex = -1;
358+
if (catchParameter instanceof OperatorNode catchOp
359+
&& "$@%".contains(catchOp.operator)
360+
&& catchOp.operand instanceof IdentifierNode catchId) {
361+
catchScopeIndex = parser.ctx.symbolTable.enterScope();
362+
parser.ctx.symbolTable.addVariable(catchOp.operator + catchId.name, "my", null);
363+
}
364+
365+
try {
357366
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "{");
358-
finallyBlock = ParseBlock.parseBlock(parser);
367+
Node catchBlock = ParseBlock.parseBlock(parser);
359368
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "}");
360-
}
361369

362-
return new BinaryOperatorNode("->",
363-
new SubroutineNode(null, null, null,
364-
new BlockNode(List.of(
370+
// Parse the optional finally block
371+
Node finallyBlock = null;
372+
if (TokenUtils.peek(parser).text.equals("finally")) {
373+
TokenUtils.consume(parser, LexerTokenType.IDENTIFIER); // "finally"
374+
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "{");
375+
finallyBlock = ParseBlock.parseBlock(parser);
376+
TokenUtils.consume(parser, LexerTokenType.OPERATOR, "}");
377+
}
378+
379+
return new BinaryOperatorNode("->",
380+
new SubroutineNode(null, null, null,
381+
new BlockNode(List.of(
365382
new TryNode(tryBlock, catchParameter, catchBlock, finallyBlock, index)), index),
366383
false, index),
367384
atUnderscoreArgs(parser),
368385
index);
386+
} finally {
387+
if (catchScopeIndex >= 0) {
388+
parser.ctx.symbolTable.exitScope(catchScopeIndex);
389+
}
390+
}
369391
}
370392

371393
/**

‎src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,11 +562,19 @@ public static Node parseSubroutineDefinition(Parser parser, boolean wantName, St
562562
}
563563

564564
ListNode signature = null;
565+
// Scope index for signature parameter variables (for strict vars checking).
566+
// Entered before parseSignature() so that default value expressions can
567+
// reference earlier parameters, and exited after the block body is parsed.
568+
int signatureScopeIndex = -1;
565569

566570
// Check if the next token is an opening parenthesis '(' indicating a prototype.
567571
if (peek(parser).text.equals("(")) {
568572
if (parser.ctx.symbolTable.isFeatureCategoryEnabled("signatures")) {
569573
if (CompilerOptions.DEBUG_ENABLED) parser.ctx.logDebug("Signatures feature enabled");
574+
// Enter a scope for signature parameter variables so the parse-time
575+
// strict vars check can find them. SignatureParser.parseParameter()
576+
// registers each parameter directly in this scope.
577+
signatureScopeIndex = parser.ctx.symbolTable.enterScope();
570578
// If the signatures feature is enabled, we parse a signature.
571579
signature = parseSignature(parser, subName);
572580
if (CompilerOptions.DEBUG_ENABLED) parser.ctx.logDebug("Signature AST: " + signature);
@@ -727,6 +735,10 @@ public static Node parseSubroutineDefinition(Parser parser, boolean wantName, St
727735
return handleNamedSub(parser, subName, prototype, attributes, block, declaration);
728736
}
729737
} finally {
738+
// Exit the signature scope if we entered one
739+
if (signatureScopeIndex >= 0) {
740+
parser.ctx.symbolTable.exitScope(signatureScopeIndex);
741+
}
730742
// Restore the previous subroutine context
731743
parser.ctx.symbolTable.setCurrentSubroutine(previousSubroutine);
732744
parser.ctx.symbolTable.setInSubroutineBody(previousInSubroutineBody);

‎src/main/java/org/perlonjava/frontend/parser/Variable.java‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.perlonjava.frontend.lexer.LexerTokenType;
88
import org.perlonjava.frontend.semantic.SymbolTable;
99
import org.perlonjava.runtime.operators.WarnDie;
10+
import org.perlonjava.runtime.perlmodule.Strict;
1011
import org.perlonjava.runtime.runtimetypes.*;
1112

1213
import java.util.ArrayList;
@@ -239,6 +240,10 @@ && isFieldInClassHierarchy(parser, varName)
239240
}
240241
}
241242

243+
// Check strict vars at parse time — catches undeclared variables in
244+
// lazily-compiled named sub bodies that would otherwise be missed
245+
checkStrictVarsAtParseTime(parser, sigil, varName);
246+
242247
// Normal variable: create a simple variable reference node
243248
return new OperatorNode(sigil, new IdentifierNode(varName, parser.tokenIndex), parser.tokenIndex);
244249
} else if (peek(parser).text.equals("{")) {
@@ -252,6 +257,114 @@ && isFieldInClassHierarchy(parser, varName)
252257
return new OperatorNode(sigil, operand, parser.tokenIndex);
253258
}
254259

260+
/**
261+
* Check strict vars at parse time for an unqualified variable.
262+
* This catches undeclared variables inside lazily-compiled named sub bodies
263+
* that would otherwise only be detected at call time (or never).
264+
* Mirrors the exemption logic from EmitVariable.java and BytecodeCompiler.java.
265+
*/
266+
private static void checkStrictVarsAtParseTime(Parser parser, String sigil, String varName) {
267+
// Only check $, @, % sigils (not *, &, $#)
268+
if (!sigil.equals("$") && !sigil.equals("@") && !sigil.equals("%")) return;
269+
270+
// Skip when parsing a my/our/state declaration — the variable is being declared
271+
if (parser.parsingDeclaration) return;
272+
273+
// Check if strict vars is enabled in the current scope
274+
if (!parser.ctx.symbolTable.isStrictOptionEnabled(Strict.HINT_STRICT_VARS)) return;
275+
276+
// Variable declared lexically (my, our, state) — always allowed
277+
if (parser.ctx.symbolTable.getSymbolEntry(sigil + varName) != null) return;
278+
279+
// For $name{...} (hash element) or $name[...] (array element), check the
280+
// container variable too: $hash{key} is valid if %hash is declared,
281+
// and $array[0] is valid if @array is declared.
282+
// Similarly, @name{...} is a hash slice (valid if %name is declared).
283+
if (sigil.equals("$") || sigil.equals("@")) {
284+
int peekIdx = Whitespace.skipWhitespace(parser, parser.tokenIndex, parser.tokens);
285+
if (peekIdx < parser.tokens.size()) {
286+
String nextText = parser.tokens.get(peekIdx).text;
287+
if (nextText.equals("{") && parser.ctx.symbolTable.getSymbolEntry("%" + varName) != null) return;
288+
if (nextText.equals("[") && parser.ctx.symbolTable.getSymbolEntry("@" + varName) != null) return;
289+
}
290+
}
291+
292+
// Qualified names (Pkg::var) — always allowed
293+
if (varName.contains("::")) return;
294+
295+
// Regex capture variables ($1, $2, ...) but not $01, $02
296+
if (ScalarUtils.isInteger(varName) && !varName.startsWith("0")) return;
297+
298+
// Sort variables $a and $b
299+
if (sigil.equals("$") && (varName.equals("a") || varName.equals("b"))) return;
300+
301+
// Built-in special length-one vars ($_, $!, $;, $0, etc.)
302+
if (sigil.equals("$") && varName.length() == 1 && !Character.isLetter(varName.charAt(0))) return;
303+
304+
// Built-in special scalar vars (${^GLOBAL_PHASE}, $ARGV, $STDIN, etc.)
305+
if (sigil.equals("$") && !varName.isEmpty() && varName.charAt(0) < 32) return;
306+
if (sigil.equals("$") && (varName.equals("ARGV") || varName.equals("ARGVOUT")
307+
|| varName.equals("ENV") || varName.equals("INC") || varName.equals("SIG")
308+
|| varName.equals("STDIN") || varName.equals("STDOUT") || varName.equals("STDERR"))) return;
309+
310+
// Built-in special container vars (%ENV, %SIG, @ARGV, @INC, etc.)
311+
if (sigil.equals("%") && (varName.equals("SIG") || varName.equals("ENV")
312+
|| varName.equals("INC") || varName.equals("+") || varName.equals("-"))) return;
313+
if (sigil.equals("@") && (varName.equals("ARGV") || varName.equals("INC")
314+
|| varName.equals("_") || varName.equals("F"))) return;
315+
316+
// Non-ASCII length-1 scalars under 'no utf8' (Latin-1 range)
317+
if (sigil.equals("$") && varName.length() == 1) {
318+
char c = varName.charAt(0);
319+
if (c > 127 && c <= 255
320+
&& !parser.ctx.symbolTable.isStrictOptionEnabled(Strict.HINT_UTF8)) return;
321+
}
322+
323+
// Check if variable already exists in the global registry (from use vars, etc.)
324+
String normalizedName = NameNormalizer.normalizeVariableName(
325+
varName, parser.ctx.symbolTable.getCurrentPackage());
326+
boolean existsGlobally = false;
327+
if (sigil.equals("$")) {
328+
existsGlobally = GlobalVariable.existsGlobalVariable(normalizedName);
329+
// For $hash{...} and $array[...], also check global container
330+
if (!existsGlobally) {
331+
int peekIdx = Whitespace.skipWhitespace(parser, parser.tokenIndex, parser.tokens);
332+
if (peekIdx < parser.tokens.size()) {
333+
String nextText = parser.tokens.get(peekIdx).text;
334+
if (nextText.equals("{") && GlobalVariable.existsGlobalHash(normalizedName)) existsGlobally = true;
335+
if (nextText.equals("[") && GlobalVariable.existsGlobalArray(normalizedName)) existsGlobally = true;
336+
}
337+
}
338+
} else if (sigil.equals("@")) {
339+
existsGlobally = GlobalVariable.existsGlobalArray(normalizedName);
340+
// For @hash{...} (hash slice), also check global hash
341+
if (!existsGlobally) {
342+
int peekIdx = Whitespace.skipWhitespace(parser, parser.tokenIndex, parser.tokens);
343+
if (peekIdx < parser.tokens.size()) {
344+
String nextText = parser.tokens.get(peekIdx).text;
345+
if (nextText.equals("{") && GlobalVariable.existsGlobalHash(normalizedName)) existsGlobally = true;
346+
}
347+
}
348+
} else if (sigil.equals("%") && !normalizedName.endsWith("::"))
349+
existsGlobally = GlobalVariable.existsGlobalHash(normalizedName);
350+
351+
// Single-letter scalars require declaration even if they exist globally
352+
if (sigil.equals("$") && varName.length() == 1
353+
&& Character.isLetter(varName.charAt(0))
354+
&& !varName.equals("a") && !varName.equals("b")) {
355+
existsGlobally = false;
356+
}
357+
358+
if (existsGlobally) return;
359+
360+
// Undeclared variable under strict vars
361+
throw new PerlCompilerException(parser.tokenIndex,
362+
"Global symbol \"" + sigil + varName
363+
+ "\" requires explicit package name (did you forget to declare \"my "
364+
+ sigil + varName + "\"?)",
365+
parser.ctx.errorUtil);
366+
}
367+
255368
/**
256369
* Parses array and hash access operations within braces (for ${...} constructs).
257370
* This is similar to parseArrayHashAccess but stops at the closing brace.

0 commit comments

Comments
 (0)