Skip to content

Commit b963522

Browse files
committed
[GR-23260] pass more test_sort tests
PullRequest: graalpython/1015
2 parents cdc2093 + 3255fd0 commit b963522

File tree

17 files changed

+176
-63
lines changed

17 files changed

+176
-63
lines changed

graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_sort.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,5 @@
1313
*graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_long_compare
1414
*graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_object_compare
1515
*graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_tuple_compare
16+
*graalpython.lib-python.3.test.test_sort.TestBugs.test_bug453523
17+
*graalpython.lib-python.3.test.test_sort.TestBugs.test_undetected_mutation

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,14 +1809,14 @@ public PSequence rangeStartStopStep(Object cls, Object start, Object stop, Objec
18091809
}
18101810
}
18111811

1812-
throw raise(TypeError, ErrorMessages.RAGE_DOES_NOT_SUPPORT, start, stop, step);
1812+
throw raise(TypeError, ErrorMessages.RANGE_DOES_NOT_SUPPORT, start, stop, step);
18131813
}
18141814

18151815
@TruffleBoundary
18161816
@Specialization(guards = "!isNumber(stop)")
18171817
public PSequence rangeError(Object cls, Object start, Object stop, Object step) {
18181818
CompilerDirectives.transferToInterpreter();
1819-
throw raise(TypeError, ErrorMessages.RAGE_DOES_NOT_SUPPORT, start, stop, step);
1819+
throw raise(TypeError, ErrorMessages.RANGE_DOES_NOT_SUPPORT, start, stop, step);
18201820
}
18211821

18221822
public static boolean isNumber(Object value) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PythonCextBuiltins.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2609,7 +2609,7 @@ Object doPTuple(VirtualFrame frame, PTuple tuple, long key,
26092609
SequenceStorage sequenceStorage = tuple.getSequenceStorage();
26102610
// we must do a bounds-check but we must not normalize the index
26112611
if (key < 0 || key >= lenNode.execute(sequenceStorage)) {
2612-
throw raise(IndexError, NormalizeIndexNode.TUPLE_OUT_OF_BOUNDS);
2612+
throw raise(IndexError, ErrorMessages.TUPLE_OUT_OF_BOUNDS);
26132613
}
26142614
return getItemNode.execute(frame, sequenceStorage, key);
26152615
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/IndexNodes.java

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.oracle.graal.python.builtins.objects.common.IndexNodesFactory.NormalizeIndexWithBoundsCheckNodeGen;
4646
import com.oracle.graal.python.builtins.objects.common.IndexNodesFactory.NormalizeIndexWithoutBoundsCheckNodeGen;
4747
import com.oracle.graal.python.builtins.objects.ints.PInt;
48+
import com.oracle.graal.python.nodes.ErrorMessages;
4849
import com.oracle.graal.python.nodes.PRaiseNode;
4950
import com.oracle.graal.python.util.OverflowException;
5051
import com.oracle.truffle.api.dsl.Cached;
@@ -57,15 +58,6 @@
5758
public abstract class IndexNodes {
5859

5960
public static final class NormalizeIndexNode extends Node {
60-
public static final String INDEX_OUT_OF_BOUNDS = "index out of range";
61-
public static final String RANGE_OUT_OF_BOUNDS = "range index out of range";
62-
public static final String TUPLE_OUT_OF_BOUNDS = "tuple index out of range";
63-
public static final String TUPLE_ASSIGN_OUT_OF_BOUNDS = "tuple assignment index out of range";
64-
public static final String LIST_OUT_OF_BOUNDS = "list index out of range";
65-
public static final String LIST_ASSIGN_OUT_OF_BOUNDS = "list assignment index out of range";
66-
public static final String ARRAY_OUT_OF_BOUNDS = "array index out of range";
67-
public static final String ARRAY_ASSIGN_OUT_OF_BOUNDS = "array assignment index out of range";
68-
public static final String BYTEARRAY_OUT_OF_BOUNDS = "bytearray index out of range";
6961

7062
@Child private NormalizeIndexCustomMessageNode subNode;
7163

@@ -101,51 +93,51 @@ protected final String getErrorMessage() {
10193
}
10294

10395
public static NormalizeIndexNode create() {
104-
return new NormalizeIndexNode(INDEX_OUT_OF_BOUNDS, true);
96+
return new NormalizeIndexNode(ErrorMessages.INDEX_OUT_OF_RANGE, true);
10597
}
10698

10799
public static NormalizeIndexNode create(String errorMessage) {
108100
return new NormalizeIndexNode(errorMessage, true);
109101
}
110102

111103
public static NormalizeIndexNode create(boolean boundsCheck) {
112-
return new NormalizeIndexNode(INDEX_OUT_OF_BOUNDS, boundsCheck);
104+
return new NormalizeIndexNode(ErrorMessages.INDEX_OUT_OF_RANGE, boundsCheck);
113105
}
114106

115107
public static NormalizeIndexNode create(String errorMessage, boolean boundsCheck) {
116108
return new NormalizeIndexNode(errorMessage, boundsCheck);
117109
}
118110

119111
public static NormalizeIndexNode forList() {
120-
return create(LIST_OUT_OF_BOUNDS);
112+
return create(ErrorMessages.LIST_INDEX_OUT_OF_RANGE);
121113
}
122114

123115
public static NormalizeIndexNode forListAssign() {
124-
return create(LIST_ASSIGN_OUT_OF_BOUNDS);
116+
return create(ErrorMessages.LIST_ASSIGMENT_INDEX_OUT_OF_RANGE);
125117
}
126118

127119
public static NormalizeIndexNode forTuple() {
128-
return create(TUPLE_OUT_OF_BOUNDS);
120+
return create(ErrorMessages.TUPLE_OUT_OF_BOUNDS);
129121
}
130122

131123
public static NormalizeIndexNode forTupleAssign() {
132-
return create(TUPLE_ASSIGN_OUT_OF_BOUNDS);
124+
return create(ErrorMessages.TUPLE_ASSIGN_OUT_OF_BOUNDS);
133125
}
134126

135127
public static NormalizeIndexNode forArray() {
136-
return create(ARRAY_OUT_OF_BOUNDS);
128+
return create(ErrorMessages.ARRAY_OUT_OF_BOUNDS);
137129
}
138130

139131
public static NormalizeIndexNode forArrayAssign() {
140-
return create(ARRAY_ASSIGN_OUT_OF_BOUNDS);
132+
return create(ErrorMessages.ARRAY_ASSIGN_OUT_OF_BOUNDS);
141133
}
142134

143135
public static NormalizeIndexNode forRange() {
144-
return create(RANGE_OUT_OF_BOUNDS);
136+
return create(ErrorMessages.RANGE_OUT_OF_BOUNDS);
145137
}
146138

147139
public static NormalizeIndexNode forBytearray() {
148-
return create(BYTEARRAY_OUT_OF_BOUNDS);
140+
return create(ErrorMessages.BYTEARRAY_OUT_OF_BOUNDS);
149141
}
150142
}
151143

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import static com.oracle.graal.python.builtins.objects.cext.NativeCAPISymbols.FUN_PY_TRUFFLE_INT_ARRAY_TO_NATIVE;
3131
import static com.oracle.graal.python.builtins.objects.cext.NativeCAPISymbols.FUN_PY_TRUFFLE_LONG_ARRAY_TO_NATIVE;
3232
import static com.oracle.graal.python.builtins.objects.cext.NativeCAPISymbols.FUN_PY_TRUFFLE_OBJECT_ARRAY_TO_NATIVE;
33-
import static com.oracle.graal.python.builtins.objects.common.IndexNodes.NormalizeIndexNode.INDEX_OUT_OF_BOUNDS;
3433
import static com.oracle.graal.python.nodes.SpecialMethodNames.__EQ__;
3534
import static com.oracle.graal.python.nodes.SpecialMethodNames.__GE__;
3635
import static com.oracle.graal.python.nodes.SpecialMethodNames.__GT__;
@@ -621,31 +620,31 @@ protected Object doScalarInt(@SuppressWarnings("unused") ContainerFactory factor
621620
@Shared("getItemScalarNode") @Cached GetItemScalarNode getItemScalarNode,
622621
@Shared("normalizeIndexNode") @Cached NormalizeIndexCustomMessageNode normalizeIndexNode,
623622
@Shared("lenNode") @Cached LenNode lenNode) {
624-
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS));
623+
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE));
625624
}
626625

627626
@Specialization
628627
protected Object doScalarLong(@SuppressWarnings("unused") ContainerFactory factoryMethod, SequenceStorage storage, long idx,
629628
@Shared("getItemScalarNode") @Cached GetItemScalarNode getItemScalarNode,
630629
@Shared("normalizeIndexNode") @Cached NormalizeIndexCustomMessageNode normalizeIndexNode,
631630
@Shared("lenNode") @Cached LenNode lenNode) {
632-
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS));
631+
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE));
633632
}
634633

635634
@Specialization
636635
protected Object doScalarPInt(@SuppressWarnings("unused") ContainerFactory factoryMethod, SequenceStorage storage, PInt idx,
637636
@Shared("getItemScalarNode") @Cached GetItemScalarNode getItemScalarNode,
638637
@Shared("normalizeIndexNode") @Cached NormalizeIndexCustomMessageNode normalizeIndexNode,
639638
@Shared("lenNode") @Cached LenNode lenNode) {
640-
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS));
639+
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE));
641640
}
642641

643642
@Specialization(guards = "!isPSlice(idx)")
644643
protected Object doScalarGeneric(@SuppressWarnings("unused") ContainerFactory factoryMethod, SequenceStorage storage, Object idx,
645644
@Shared("getItemScalarNode") @Cached GetItemScalarNode getItemScalarNode,
646645
@Shared("normalizeIndexNode") @Cached NormalizeIndexCustomMessageNode normalizeIndexNode,
647646
@Shared("lenNode") @Cached LenNode lenNode) {
648-
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS));
647+
return getItemScalarNode.execute(storage, normalizeIndexNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE));
649648
}
650649

651650
@Specialization
@@ -912,7 +911,7 @@ protected static SequenceStorage doScalarInt(GenNodeSupplier generalizationNodeP
912911
@Shared("doGenNode") @Cached DoGeneralizationNode doGenNode,
913912
@Shared("normalizeNode") @Cached NormalizeIndexCustomMessageNode normalizeNode,
914913
@Shared("lenNode") @Cached LenNode lenNode) {
915-
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS);
914+
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE);
916915
try {
917916
setItemScalarNode.execute(storage, normalized, value);
918917
return storage;
@@ -936,7 +935,7 @@ protected static SequenceStorage doScalarLong(GenNodeSupplier generalizationNode
936935
@Shared("doGenNode") @Cached DoGeneralizationNode doGenNode,
937936
@Shared("normalizeNode") @Cached NormalizeIndexCustomMessageNode normalizeNode,
938937
@Shared("lenNode") @Cached LenNode lenNode) {
939-
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS);
938+
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE);
940939
try {
941940
setItemScalarNode.execute(storage, normalized, value);
942941
return storage;
@@ -955,7 +954,7 @@ protected static SequenceStorage doScalarPInt(GenNodeSupplier generalizationNode
955954
@Shared("doGenNode") @Cached DoGeneralizationNode doGenNode,
956955
@Shared("normalizeNode") @Cached NormalizeIndexCustomMessageNode normalizeNode,
957956
@Shared("lenNode") @Cached LenNode lenNode) {
958-
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS);
957+
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE);
959958
try {
960959
setItemScalarNode.execute(storage, normalized, value);
961960
return storage;
@@ -974,7 +973,7 @@ protected static SequenceStorage doScalarGeneric(GenNodeSupplier generalizationN
974973
@Shared("doGenNode") @Cached DoGeneralizationNode doGenNode,
975974
@Shared("normalizeNode") @Cached NormalizeIndexCustomMessageNode normalizeNode,
976975
@Shared("lenNode") @Cached LenNode lenNode) {
977-
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), INDEX_OUT_OF_BOUNDS);
976+
int normalized = normalizeNode.execute(idx, lenNode.execute(storage), ErrorMessages.INDEX_OUT_OF_RANGE);
978977
try {
979978
setItemScalarNode.execute(storage, normalized, value);
980979
return storage;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
6666
import com.oracle.graal.python.builtins.objects.function.PArguments;
6767
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
68+
import com.oracle.graal.python.builtins.objects.function.PKeyword;
6869
import com.oracle.graal.python.builtins.objects.generator.PGenerator;
6970
import com.oracle.graal.python.builtins.objects.ints.PInt;
7071
import com.oracle.graal.python.builtins.objects.iterator.PDoubleSequenceIterator;
@@ -80,9 +81,11 @@
8081
import com.oracle.graal.python.nodes.ErrorMessages;
8182
import com.oracle.graal.python.nodes.PGuards;
8283
import com.oracle.graal.python.nodes.argument.ReadArgumentNode;
84+
import com.oracle.graal.python.nodes.attributes.GetAttributeNode;
8385
import com.oracle.graal.python.nodes.builtins.ListNodes;
8486
import com.oracle.graal.python.nodes.builtins.ListNodes.AppendNode;
8587
import com.oracle.graal.python.nodes.builtins.ListNodes.IndexNode;
88+
import com.oracle.graal.python.nodes.call.CallNode;
8689
import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
8790
import com.oracle.graal.python.nodes.control.GetIteratorExpressionNode.GetIteratorNode;
8891
import com.oracle.graal.python.nodes.control.GetNextNode;
@@ -91,6 +94,7 @@
9194
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
9295
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
9396
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
97+
import com.oracle.graal.python.nodes.function.builtins.PythonVarargsBuiltinNode;
9498
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
9599
import com.oracle.graal.python.nodes.truffle.PythonArithmeticTypes;
96100
import com.oracle.graal.python.runtime.PythonCore;
@@ -419,6 +423,20 @@ public static ListExtendNode create() {
419423
}
420424
}
421425

426+
// list.copy()
427+
@Builtin(name = "copy", minNumOfPositionalArgs = 1)
428+
@GenerateNodeFactory
429+
public abstract static class ListCopyNode extends PythonUnaryBuiltinNode {
430+
431+
@Specialization(limit = "3")
432+
PList copySequence(PList self,
433+
@Cached SequenceStorageNodes.CopyNode copy,
434+
@CachedLibrary("self") PythonObjectLibrary plib) {
435+
return factory().createList(plib.getLazyPythonClass(self), copy.execute(self.getSequenceStorage()));
436+
}
437+
438+
}
439+
422440
// list.insert(i, x)
423441
@Builtin(name = "insert", minNumOfPositionalArgs = 3)
424442
@GenerateNodeFactory
@@ -826,6 +844,65 @@ public static ListReverseNode create() {
826844
}
827845
}
828846

847+
// list.sort(key=, reverse=)
848+
@Builtin(name = "sort", minNumOfPositionalArgs = 1, takesVarArgs = true, takesVarKeywordArgs = true, needsFrame = true)
849+
@GenerateNodeFactory
850+
public abstract static class ListSortNode extends PythonVarargsBuiltinNode {
851+
852+
protected static final String SORT = "_sort";
853+
protected static final String KEY = "key";
854+
855+
protected static boolean isSortable(PList list, SequenceStorageNodes.LenNode lenNode) {
856+
return lenNode.execute(list.getSequenceStorage()) > 1;
857+
}
858+
859+
protected static boolean maySideEffect(PList list, PKeyword[] keywords) {
860+
if (PGuards.isObjectStorage(list)) {
861+
return true;
862+
}
863+
if (keywords.length > 0) {
864+
if (keywords[0].getName().equals(KEY)) {
865+
return true;
866+
}
867+
if (keywords.length > 1 && keywords[1].getName().equals(KEY)) {
868+
return true;
869+
}
870+
}
871+
return false;
872+
}
873+
874+
@Specialization(guards = "!isSortable(list, lenNode)")
875+
@SuppressWarnings("unused")
876+
Object none(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
877+
@Cached SequenceStorageNodes.LenNode lenNode) {
878+
return PNone.NONE;
879+
}
880+
881+
@Specialization(guards = {"isSortable(list, lenNode)", "maySideEffect(list, keywords)"})
882+
Object withKey(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
883+
@Cached("create(SORT)") GetAttributeNode sort,
884+
@Cached CallNode callSort,
885+
@SuppressWarnings("unused") @Cached SequenceStorageNodes.LenNode lenNode) {
886+
list.getSequenceStorage().setLock();
887+
try {
888+
defaultSort(frame, list, arguments, keywords, sort, callSort, lenNode);
889+
} finally {
890+
list.getSequenceStorage().releaseLock();
891+
}
892+
return PNone.NONE;
893+
}
894+
895+
@Specialization(guards = {"isSortable(list, lenNode)", "!maySideEffect(list, keywords)"})
896+
Object defaultSort(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
897+
@Cached("create(SORT)") GetAttributeNode sort,
898+
@Cached CallNode callSort,
899+
@SuppressWarnings("unused") @Cached SequenceStorageNodes.LenNode lenNode) {
900+
Object sortMethod = sort.executeObject(frame, list);
901+
callSort.execute(sortMethod, arguments, keywords);
902+
return PNone.NONE;
903+
}
904+
}
905+
829906
@Builtin(name = __LEN__, minNumOfPositionalArgs = 1)
830907
@GenerateNodeFactory
831908
public abstract static class LenNode extends PythonUnaryBuiltinNode {

0 commit comments

Comments
 (0)