Skip to content

Commit

Permalink
Fix unbound local checks, remove unused/redundant operations
Browse files Browse the repository at this point in the history
  • Loading branch information
DSouzaM committed Oct 11, 2024
1 parent 777c74d commit 3063e4f
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 156 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.oracle.graal.python.builtins.objects.ellipsis.PEllipsis;
import com.oracle.graal.python.builtins.objects.function.PArguments;
import com.oracle.graal.python.builtins.objects.function.PKeyword;
import com.oracle.graal.python.builtins.objects.type.TypeFlags;
import com.oracle.graal.python.compiler.CompilationScope;
import com.oracle.graal.python.compiler.Compiler;
import com.oracle.graal.python.compiler.Compiler.ConstantCollection;
Expand Down Expand Up @@ -845,7 +846,11 @@ private BytecodeDSLCompilerResult buildComprehensionCodeUnit(SSTNode node, Compr
@Override
public BytecodeDSLCompilerResult visit(ExprTy.ListComp node) {
return buildComprehensionCodeUnit(node, node.generators, "<listcomp>",
(statementCompiler) -> statementCompiler.b.emitMakeEmptyList(),
(statementCompiler) -> {
statementCompiler.b.beginMakeList();
statementCompiler.b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
statementCompiler.b.endMakeList();
},
(statementCompiler, collection) -> {
statementCompiler.b.beginListAppend();
statementCompiler.b.emitLoadLocal(collection);
Expand All @@ -857,7 +862,10 @@ public BytecodeDSLCompilerResult visit(ExprTy.ListComp node) {
@Override
public BytecodeDSLCompilerResult visit(ExprTy.DictComp node) {
return buildComprehensionCodeUnit(node, node.generators, "<dictcomp>",
(statementCompiler) -> statementCompiler.b.emitMakeEmptyDict(),
(statementCompiler) -> {
statementCompiler.b.beginMakeDict(0);
statementCompiler.b.endMakeDict();
},
(statementCompiler, collection) -> {
statementCompiler.b.beginSetDictItem();
statementCompiler.b.emitLoadLocal(collection);
Expand All @@ -870,7 +878,11 @@ public BytecodeDSLCompilerResult visit(ExprTy.DictComp node) {
@Override
public BytecodeDSLCompilerResult visit(ExprTy.SetComp node) {
return buildComprehensionCodeUnit(node, node.generators, "<setcomp>",
(statementCompiler) -> statementCompiler.b.emitMakeEmptySet(),
(statementCompiler) -> {
statementCompiler.b.beginMakeSet();
statementCompiler.b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
statementCompiler.b.endMakeSet();
},
(statementCompiler, collection) -> {
statementCompiler.b.beginSetAdd();
statementCompiler.b.emitLoadLocal(collection);
Expand Down Expand Up @@ -968,21 +980,14 @@ private void emitNameFastOperation(String mangled, NameOperation op, Builder b)
BytecodeLocal local = locals.get(mangled);
switch (op) {
case Read:
b.beginCheckUnboundLocal(varnames.get(mangled));
b.emitLoadLocal(local);
b.endCheckUnboundLocal();
break;
case Delete:
b.beginBlock();
b.beginCheckUnboundLocal(varnames.get(mangled));
b.emitCheckUnboundLocal(local, varnames.get(mangled));
b.emitLoadLocal(local);
b.endCheckUnboundLocal();

b.beginStoreLocal(local);
b.emitLoadNull();
b.endStoreLocal();
b.endBlock();
break;
case Delete:
b.emitDeleteLocal(local, varnames.get(mangled));
break;
case BeginWrite:
if (local == null) {
throw new NullPointerException("local " + mangled + " not defined");
Expand Down Expand Up @@ -1142,7 +1147,7 @@ public void setUpFrame(ArgumentsTy args, Builder b) {
List<BytecodeLocal> toClear = new ArrayList<>();

b.beginStoreRange(cellVariableLocals);
b.beginMakeVariadic();
b.beginCollectToObjectArray();
for (int i = 0; i < cellVariableLocals.length; i++) {
b.beginCreateCell();
if (scope.getUseOfName(cellVariables[i]).contains(DefUse.DefParam)) {
Expand All @@ -1159,7 +1164,7 @@ public void setUpFrame(ArgumentsTy args, Builder b) {
}
b.endCreateCell();
}
b.endMakeVariadic();
b.endCollectToObjectArray();
b.endStoreRange();

for (BytecodeLocal local : toClear) {
Expand Down Expand Up @@ -1722,11 +1727,11 @@ private void createConstant(ConstantValue value) {
break;
case TUPLE:
b.beginMakeTuple();
b.beginMakeVariadic();
b.beginCollectToObjectArray();
for (ConstantValue cv : value.getTupleElements()) {
createConstant(cv);
}
b.endMakeVariadic();
b.endCollectToObjectArray();
b.endMakeTuple();
break;
case FROZENSET:
Expand Down Expand Up @@ -1767,7 +1772,8 @@ public Void visit(ExprTy.Dict node) {
boolean newStatement = beginSourceSection(node, b);

if (len(node.keys) == 0) {
b.emitMakeEmptyDict();
b.beginMakeDict(0);
b.endMakeDict();
} else {
b.beginMakeDict(node.keys.length);
for (int i = 0; i < node.keys.length; i++) {
Expand Down Expand Up @@ -2027,11 +2033,11 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
*
* @formatter:off
* Unstar(
* MakeVariadic(a, b),
* CollectToObjectArray(a, b),
* UnpackStarred(c),
* MakeVariadic(d, e),
* CollectToObjectArray(d, e),
* UnpackStarred(f),
* MakeVariadic(g)
* CollectToObjectArray(g)
* )
* @formatter:on
*/
Expand All @@ -2040,15 +2046,15 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
int numOperands = 0;

if (initialElementsProducer != null) {
b.beginMakeVariadic();
b.beginCollectToObjectArray();
initialElementsProducer.run();
inVariadic = true;
}

for (int i = 0; i < args.length; i++) {
if (args[i] instanceof ExprTy.Starred) {
if (inVariadic) {
b.endMakeVariadic();
b.endCollectToObjectArray();
inVariadic = false;
numOperands++;
}
Expand All @@ -2059,7 +2065,7 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
numOperands++;
} else {
if (!inVariadic) {
b.beginMakeVariadic();
b.beginCollectToObjectArray();
inVariadic = true;
}

Expand All @@ -2068,33 +2074,31 @@ private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
}

if (inVariadic) {
b.endMakeVariadic();
b.endCollectToObjectArray();
numOperands++;
}

b.endUnstar(numOperands);
} else {
b.beginMakeVariadic();
b.beginCollectToObjectArray();
if (initialElementsProducer != null) {
initialElementsProducer.run();
}
visitSequence(args);
b.endMakeVariadic();
b.endCollectToObjectArray();
}
}

@Override
public Void visit(ExprTy.Set node) {
boolean newStatement = beginSourceSection(node, b);

b.beginMakeSet();
if (len(node.elements) == 0) {
b.emitMakeEmptySet();
b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
} else {
b.beginMakeSet();
emitUnstar(node.elements);
b.endMakeSet();
}

b.endMakeSet();
endSourceSection(b, newStatement);
return null;
}
Expand Down Expand Up @@ -2943,7 +2947,8 @@ private void emitKeywordGroup(KeywordGroup group, boolean copy, BytecodeLocal fu

if (copy) {
b.beginKwargsMerge(function);
b.emitMakeEmptyDict();
b.beginMakeDict(0);
b.endMakeDict();
splatKeywords.expr.accept(this);
b.endKwargsMerge();
} else {
Expand Down Expand Up @@ -3229,11 +3234,11 @@ private void emitMakeFunction(SSTNode node, String name, ArgumentsTy args, List<
if (args == null || len(args.defaults) == 0) {
b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
} else {
b.beginMakeVariadic();
b.beginCollectToObjectArray();
for (int i = 0; i < args.defaults.length; i++) {
args.defaults[i].accept(this);
}
b.endMakeVariadic();
b.endCollectToObjectArray();
}

boolean hasKeywords = false;
Expand Down Expand Up @@ -3822,9 +3827,9 @@ private void doVisitPattern(PatternTy.MatchSequence node, PatternContext pc) {

b.beginPrimitiveBoolAnd();

b.beginIsSequence();
b.beginCheckTypeFlags(TypeFlags.SEQUENCE);
b.emitLoadLocal(pc.subject);
b.endIsSequence();
b.endCheckTypeFlags();

if (star < 0) {
// No star: len(subject) == size
Expand Down
Loading

0 comments on commit 3063e4f

Please sign in to comment.