Skip to content

Commit e33c005

Browse files
DSouzaMDSouzaM
committed
Properly report warnings during source compilation; allow exception location to be other nodes in MaterializeFrameNode
1 parent 3d73bb7 commit e33c005

File tree

7 files changed

+80
-39
lines changed

7 files changed

+80
-39
lines changed

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -674,10 +674,9 @@ private RootNode compileForBytecodeInterpreter(ModTy mod, Source source, int opt
674674
return PBytecodeRootNode.create(this, co, source, errorCallback);
675675
}
676676

677-
@SuppressWarnings("unused")
678677
private RootNode compileForBytecodeDSLInterpreter(PythonContext context, ModTy mod, Source source, int optimize,
679678
RaisePythonExceptionErrorCallback errorCallback, EnumSet<FutureFeature> futureFeatures) {
680-
BytecodeDSLCompilerResult result = BytecodeDSLCompiler.compile(this, context, mod, source, optimize, futureFeatures);
679+
BytecodeDSLCompilerResult result = BytecodeDSLCompiler.compile(this, context, mod, source, optimize, errorCallback, futureFeatures);
681680
return result.rootNode();
682681
}
683682

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@
203203
import com.oracle.graal.python.nodes.builtins.ListNodes.ConstructListNode;
204204
import com.oracle.graal.python.nodes.bytecode.GetAIterNode;
205205
import com.oracle.graal.python.nodes.bytecode.PBytecodeRootNode;
206+
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
206207
import com.oracle.graal.python.nodes.call.CallDispatchNode;
207208
import com.oracle.graal.python.nodes.call.CallNode;
208209
import com.oracle.graal.python.nodes.call.GenericInvokeNode;
@@ -1220,8 +1221,12 @@ Object generic(VirtualFrame frame, Object wSource, Object wFilename, TruffleStri
12201221

12211222
private static PCode wrapRootCallTarget(RootCallTarget rootCallTarget, PythonObjectFactory factory) {
12221223
RootNode rootNode = rootCallTarget.getRootNode();
1223-
if (rootNode instanceof PBytecodeRootNode) {
1224-
((PBytecodeRootNode) rootNode).triggerDeferredDeprecationWarnings();
1224+
if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
1225+
if (rootNode instanceof PBytecodeDSLRootNode bytecodeDSLRootNode) {
1226+
bytecodeDSLRootNode.triggerDeferredDeprecationWarnings();
1227+
}
1228+
} else if (rootNode instanceof PBytecodeRootNode bytecodeRootNode) {
1229+
bytecodeRootNode.triggerDeferredDeprecationWarnings();
12251230
}
12261231
return factory.createCode(rootCallTarget);
12271232
}

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/BytecodeDSLCompiler.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.oracle.graal.python.compiler.RaisePythonExceptionErrorCallback;
1010
import com.oracle.graal.python.nodes.bytecode_dsl.BytecodeDSLCodeUnit;
1111
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
12-
import com.oracle.graal.python.pegparser.ErrorCallback;
1312
import com.oracle.graal.python.pegparser.FutureFeature;
1413
import com.oracle.graal.python.pegparser.scope.Scope;
1514
import com.oracle.graal.python.pegparser.scope.ScopeEnvironment;
@@ -24,8 +23,8 @@ public class BytecodeDSLCompiler {
2423
public static final record BytecodeDSLCompilerResult(PBytecodeDSLRootNode rootNode, BytecodeDSLCodeUnit codeUnit) {
2524
}
2625

27-
public static BytecodeDSLCompilerResult compile(PythonLanguage language, PythonContext context, ModTy mod, Source source, int optimize, EnumSet<FutureFeature> futureFeatures) {
28-
ErrorCallback errorCallback = new RaisePythonExceptionErrorCallback(source, false);
26+
public static BytecodeDSLCompilerResult compile(PythonLanguage language, PythonContext context, ModTy mod, Source source, int optimize, RaisePythonExceptionErrorCallback errorCallback,
27+
EnumSet<FutureFeature> futureFeatures) {
2928
/**
3029
* Parse __future__ annotations before the analysis step. The analysis does extra validation
3130
* when __future__.annotations is imported.
@@ -37,7 +36,7 @@ public static BytecodeDSLCompilerResult compile(PythonLanguage language, PythonC
3736
return compiler.compile();
3837
}
3938

40-
private static int parseFuture(ModTy mod, EnumSet<FutureFeature> futureFeatures, ErrorCallback errorCallback) {
39+
private static int parseFuture(ModTy mod, EnumSet<FutureFeature> futureFeatures, RaisePythonExceptionErrorCallback errorCallback) {
4140
StmtTy[] stmts = null;
4241
if (mod instanceof ModTy.Module module) {
4342
stmts = module.body;
@@ -59,12 +58,12 @@ public static class BytecodeDSLCompilerContext {
5958
public final int optimizationLevel;
6059
public final EnumSet<FutureFeature> futureFeatures;
6160
public final int futureLineNumber;
62-
public final ErrorCallback errorCallback;
61+
public final RaisePythonExceptionErrorCallback errorCallback;
6362
public final ScopeEnvironment scopeEnvironment;
6463
public final Map<Scope, String> qualifiedNames;
6564

6665
public BytecodeDSLCompilerContext(PythonLanguage language, PythonContext context, ModTy mod, Source source, int optimizationLevel,
67-
EnumSet<FutureFeature> futureFeatures, int futureLineNumber, ErrorCallback errorCallback, ScopeEnvironment scopeEnvironment) {
66+
EnumSet<FutureFeature> futureFeatures, int futureLineNumber, RaisePythonExceptionErrorCallback errorCallback, ScopeEnvironment scopeEnvironment) {
6867
this.language = language;
6968
this.pythonContext = context;
7069
this.factory = context.factory();

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java

+52-24
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,15 @@
2525
import com.oracle.graal.python.compiler.OpCodes.CollectionBits;
2626
import com.oracle.graal.python.compiler.bytecode_dsl.BytecodeDSLCompiler.BytecodeDSLCompilerContext;
2727
import com.oracle.graal.python.compiler.bytecode_dsl.BytecodeDSLCompiler.BytecodeDSLCompilerResult;
28-
import com.oracle.graal.python.compiler.RaisePythonExceptionErrorCallback;
2928
import com.oracle.graal.python.compiler.Unparser;
3029
import com.oracle.graal.python.nodes.StringLiterals;
3130
import com.oracle.graal.python.nodes.bytecode_dsl.BytecodeDSLCodeUnit;
3231
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
3332
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNodeGen;
3433
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNodeGen.Builder;
35-
import com.oracle.graal.python.pegparser.ErrorCallback;
3634
import com.oracle.graal.python.pegparser.FutureFeature;
3735
import com.oracle.graal.python.pegparser.ErrorCallback.ErrorType;
36+
import com.oracle.graal.python.pegparser.ErrorCallback.WarningType;
3837
import com.oracle.graal.python.pegparser.scope.Scope;
3938
import com.oracle.graal.python.pegparser.scope.Scope.DefUse;
4039
import com.oracle.graal.python.pegparser.sst.AliasTy;
@@ -57,6 +56,7 @@
5756
import com.oracle.graal.python.pegparser.sst.StmtTy;
5857
import com.oracle.graal.python.pegparser.sst.UnaryOpTy;
5958
import com.oracle.graal.python.pegparser.sst.WithItemTy;
59+
import com.oracle.graal.python.pegparser.sst.ExprTy.Constant;
6060
import com.oracle.graal.python.pegparser.sst.ExprTy.DictComp;
6161
import com.oracle.graal.python.pegparser.sst.ExprTy.GeneratorExp;
6262
import com.oracle.graal.python.pegparser.sst.ExprTy.Lambda;
@@ -94,7 +94,6 @@ public class RootNodeCompiler implements BaseBytecodeDSLVisitor<BytecodeDSLCompi
9494
private final CompilationScope scopeType;
9595
private final boolean isInteractive;
9696
private final EnumSet<FutureFeature> futureFeatures;
97-
private final ErrorCallback errorCallback;
9897

9998
// Immutable after construction
10099
private final HashMap<String, Integer> varnames;
@@ -120,7 +119,6 @@ public RootNodeCompiler(BytecodeDSLCompilerContext ctx, SSTNode rootNode, EnumSe
120119
this.scopeType = getScopeType(scope, rootNode);
121120
this.isInteractive = rootNode instanceof ModTy.Interactive;
122121
this.futureFeatures = futureFeatures;
123-
this.errorCallback = new RaisePythonExceptionErrorCallback(ctx.source, false);
124122

125123
this.varnames = new HashMap<>();
126124
if (scope.isFunction()) {
@@ -261,7 +259,7 @@ flags, orderedTruffleStringArray(names),
261259
selfIndex,
262260
null,
263261
nodes);
264-
rootNode.setMetadata(codeUnit);
262+
rootNode.setMetadata(codeUnit, ctx.errorCallback);
265263
return new BytecodeDSLCompilerResult(rootNode, codeUnit);
266264
}
267265

@@ -306,12 +304,12 @@ private boolean nonEmpty() {
306304
protected final void checkForbiddenName(String id, NameOperation context) {
307305
if (context == NameOperation.BeginWrite) {
308306
if (id.equals("__debug__")) {
309-
errorCallback.onError(ErrorType.Syntax, currentLocation, "cannot assign to __debug__");
307+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "cannot assign to __debug__");
310308
}
311309
}
312310
if (context == NameOperation.Delete) {
313311
if (id.equals("__debug__")) {
314-
errorCallback.onError(ErrorType.Syntax, currentLocation, "cannot delete __debug__");
312+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "cannot delete __debug__");
315313
}
316314
}
317315
}
@@ -1225,10 +1223,10 @@ public Void visit(ExprTy.Attribute node) {
12251223
@Override
12261224
public Void visit(ExprTy.Await node) {
12271225
if (!scope.isFunction()) {
1228-
errorCallback.onError(ErrorType.Syntax, currentLocation, "'await' outside function");
1226+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "'await' outside function");
12291227
}
12301228
if (scopeType != CompilationScope.AsyncFunction && scopeType != CompilationScope.Comprehension) {
1231-
errorCallback.onError(ErrorType.Syntax, currentLocation, "'await' outside async function");
1229+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "'await' outside async function");
12321230
}
12331231

12341232
beginSourceSection(node, b);
@@ -1365,7 +1363,7 @@ protected final void validateKeywords(KeywordTy[] keywords) {
13651363
checkForbiddenName(keywords[i].arg, NameOperation.BeginWrite);
13661364
for (int j = i + 1; j < keywords.length; j++) {
13671365
if (keywords[i].arg.equals(keywords[j].arg)) {
1368-
errorCallback.onError(ErrorType.Syntax, currentLocation, "keyword argument repeated: " + keywords[i].arg);
1366+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "keyword argument repeated: " + keywords[i].arg);
13691367
}
13701368
}
13711369
}
@@ -1563,6 +1561,7 @@ private void endComparison(CmpOpTy op) {
15631561
@Override
15641562
public Void visit(ExprTy.Compare node) {
15651563
beginSourceSection(node, b);
1564+
checkCompare(node);
15661565

15671566
boolean multipleComparisons = node.comparators.length > 1;
15681567

@@ -1600,6 +1599,36 @@ public Void visit(ExprTy.Compare node) {
16001599
return null;
16011600
}
16021601

1602+
private void warn(SSTNode node, String message, Object... arguments) {
1603+
ctx.errorCallback.onWarning(WarningType.Syntax, node.getSourceRange(), message, arguments);
1604+
}
1605+
1606+
private void checkCompare(ExprTy node) {
1607+
if (!(node instanceof ExprTy.Compare compare)) {
1608+
return;
1609+
}
1610+
boolean left = checkIsArg(compare.left);
1611+
int n = compare.ops == null ? 0 : compare.ops.length;
1612+
for (int i = 0; i < n; ++i) {
1613+
CmpOpTy op = compare.ops[i];
1614+
boolean right = checkIsArg(compare.comparators[i]);
1615+
if (op == CmpOpTy.Is || op == CmpOpTy.IsNot) {
1616+
if (!right || !left) {
1617+
warn(compare, op == CmpOpTy.Is ? "\"is\" with a literal. Did you mean \"==\"?" : "\"is not\" with a literal. Did you mean \"!=\"?");
1618+
}
1619+
}
1620+
left = right;
1621+
}
1622+
}
1623+
1624+
private static boolean checkIsArg(ExprTy e) {
1625+
if (e instanceof ExprTy.Constant) {
1626+
ConstantValue.Kind kind = ((Constant) e).value.kind;
1627+
return kind == Kind.NONE || kind == Kind.BOOLEAN || kind == Kind.ELLIPSIS;
1628+
}
1629+
return true;
1630+
}
1631+
16031632
private void createConstant(ConstantValue value) {
16041633
switch (value.kind) {
16051634
case NONE:
@@ -2331,7 +2360,7 @@ public Void visit(StmtTy.AnnAssign node) {
23312360
checkAnnSubscr(subscript.slice);
23322361
}
23332362
} else {
2334-
errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "invalid node type for annotated assignment");
2363+
ctx.errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "invalid node type for annotated assignment");
23352364
}
23362365
if (!node.isSimple) {
23372366
/*
@@ -2744,10 +2773,10 @@ public Void visit(StmtTy.AsyncFor node) {
27442773
@Override
27452774
public Void visit(StmtTy.AsyncWith node) {
27462775
if (!scope.isFunction()) {
2747-
errorCallback.onError(ErrorType.Syntax, currentLocation, "'async with' outside function");
2776+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "'async with' outside function");
27482777
}
27492778
if (scopeType != CompilationScope.AsyncFunction && scopeType != CompilationScope.Comprehension) {
2750-
errorCallback.onError(ErrorType.Syntax, currentLocation, "'async with' outside async function");
2779+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "'async with' outside async function");
27512780
}
27522781
beginSourceSection(node, b);
27532782
visitWithRecurse(node.items, 0, node.body, true);
@@ -3230,7 +3259,6 @@ private void visitStatements(StmtTy[] stmts) {
32303259
@Override
32313260
public Void visit(StmtTy.If node) {
32323261
beginSourceSection(node, b);
3233-
32343262
if (node.orElse == null || node.orElse.length == 0) {
32353263
b.beginIfThen();
32363264
visitCondition(node.test);
@@ -3321,7 +3349,7 @@ public Void visit(StmtTy.Import node) {
33213349
public Void visit(StmtTy.ImportFrom node) {
33223350
beginSourceSection(node, b);
33233351
if (node.getSourceRange().startLine > ctx.futureLineNumber && "__future__".equals(node.module)) {
3324-
errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "from __future__ imports must occur at the beginning of the file");
3352+
ctx.errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "from __future__ imports must occur at the beginning of the file");
33253353
}
33263354

33273355
TruffleString tsModuleName = toTruffleStringUncached(node.module == null ? "" : node.module);
@@ -3409,7 +3437,7 @@ private BytecodeLocal allocateBindVariable(String name) {
34093437
}
34103438

34113439
private void duplicateStoreError(String name) {
3412-
errorCallback.onError(ErrorType.Syntax, currentLocation, "multiple assignments to name '%s' in pattern", name);
3440+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "multiple assignments to name '%s' in pattern", name);
34133441
}
34143442

34153443
}
@@ -3573,9 +3601,9 @@ private void doVisitPattern(PatternTy.MatchAs node, PatternContext pc) {
35733601
// If there's no pattern (e.g., _), it trivially matches. Ensure this is permitted.
35743602
if (!pc.allowIrrefutable) {
35753603
if (node.name != null) {
3576-
errorCallback.onError(ErrorType.Syntax, currentLocation, "name capture '%s' makes remaining patterns unreachable", node.name);
3604+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "name capture '%s' makes remaining patterns unreachable", node.name);
35773605
}
3578-
errorCallback.onError(ErrorType.Syntax, currentLocation, "wildcard makes remaining patterns unreachable");
3606+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "wildcard makes remaining patterns unreachable");
35793607
}
35803608
b.emitLoadConstant(true);
35813609
} else {
@@ -3638,12 +3666,12 @@ private void patternUnpackHelper(PatternTy[] patterns, PatternContext pc) {
36383666
PatternTy pattern = patterns[i];
36393667
if (pattern instanceof PatternTy.MatchStar) {
36403668
if (seenStar) {
3641-
errorCallback.onError(ErrorType.Syntax, currentLocation, "multiple starred expressions in sequence pattern");
3669+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "multiple starred expressions in sequence pattern");
36423670
}
36433671
seenStar = true;
36443672
int countAfter = n - i - 1;
36453673
if (countAfter != (byte) countAfter) {
3646-
errorCallback.onError(ErrorType.Syntax, currentLocation, "too many expressions in star-unpacking sequence pattern");
3674+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "too many expressions in star-unpacking sequence pattern");
36473675
}
36483676
// If there's a star pattern, emit UnpackEx.
36493677
b.beginUnpackEx(i, countAfter);
@@ -3724,7 +3752,7 @@ private void doVisitPattern(PatternTy.MatchSequence node, PatternContext pc) {
37243752
PatternTy pattern = node.patterns[i];
37253753
if (pattern instanceof PatternTy.MatchStar) {
37263754
if (star >= 0) {
3727-
errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "multiple starred names in sequence pattern");
3755+
ctx.errorCallback.onError(ErrorType.Syntax, node.getSourceRange(), "multiple starred names in sequence pattern");
37283756
}
37293757
starWildcard = wildcardStarCheck(pattern);
37303758
onlyWildcard &= starWildcard;
@@ -3820,7 +3848,7 @@ private void doVisitPattern(PatternTy.MatchValue node, PatternContext pc) {
38203848
} else if (node.value instanceof ExprTy.Constant || node.value instanceof ExprTy.Attribute) {
38213849
node.value.accept(this);
38223850
} else {
3823-
errorCallback.onError(ErrorType.Syntax, currentLocation, "patterns may only match literals and attribute lookups");
3851+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "patterns may only match literals and attribute lookups");
38243852
}
38253853
b.endEq();
38263854
}
@@ -3955,7 +3983,7 @@ public Void visit(StmtTy.Raise node) {
39553983
@Override
39563984
public Void visit(StmtTy.Return node) {
39573985
if (!scope.isFunction()) {
3958-
errorCallback.onError(ErrorType.Syntax, currentLocation, "'return' outside function");
3986+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "'return' outside function");
39593987
}
39603988

39613989
beginSourceSection(node, b);
@@ -4147,7 +4175,7 @@ private void emitTryExceptElse(StmtTy.Try node) {
41474175
for (ExceptHandlerTy h : node.handlers) {
41484176
beginSourceSection(h, b);
41494177
if (bareExceptRange != null) {
4150-
errorCallback.onError(ErrorType.Syntax, currentLocation, "default 'except:' must be last");
4178+
ctx.errorCallback.onError(ErrorType.Syntax, currentLocation, "default 'except:' must be last");
41514179
}
41524180

41534181
ExceptHandlerTy.ExceptHandler handler = (ExceptHandlerTy.ExceptHandler) h;

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/BytecodeDSLCodeUnit.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public PBytecodeDSLRootNode createRootNode(PythonContext context, Source source)
5151
BytecodeRootNodes<PBytecodeDSLRootNode> deserialized = MarshalModuleBuiltins.deserializeBytecodeNodes(context.getLanguage(), source, toDeserialize);
5252
assert deserialized.count() == 1;
5353
PBytecodeDSLRootNode result = deserialized.getNode(0);
54-
result.setMetadata(this);
54+
result.setMetadata(this, null);
5555
return result;
5656
}
5757

0 commit comments

Comments
 (0)