Skip to content

Commit 65265fb

Browse files
steve-ssteve-s
committed
[GR-18876] Move the unpacking logic to StarredExpression.
PullRequest: graalpython/938
2 parents 72cb697 + 668b708 commit 65265fb

File tree

12 files changed

+284
-155
lines changed

12 files changed

+284
-155
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_starred_expr.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# The Universal Permissive License (UPL), Version 1.0
5+
#
6+
# Subject to the condition set forth below, permission is hereby granted to any
7+
# person obtaining a copy of this software, associated documentation and/or
8+
# data (collectively the "Software"), free of charge and under any and all
9+
# copyright rights in the Software, and any and all patent rights owned or
10+
# freely licensable by each licensor hereunder covering either (i) the
11+
# unmodified Software as contributed to or provided by such licensor, or (ii)
12+
# the Larger Works (as defined below), to deal in both
13+
#
14+
# (a) the Software, and
15+
#
16+
# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
17+
# one is included with the Software each a "Larger Work" to which the Software
18+
# is contributed by such licensors),
19+
#
20+
# without restriction, including without limitation the rights to copy, create
21+
# derivative works of, display, perform, and distribute the Software and make,
22+
# use, sell, offer for sale, import, export, have made, and have sold the
23+
# Software and the Larger Work(s), and to sublicense the foregoing rights on
24+
# either these or other terms.
25+
#
26+
# This license is subject to the following condition:
27+
#
28+
# The above copyright notice and either this complete permission notice or at a
29+
# minimum a reference to the UPL must be included in all copies or substantial
30+
# portions of the Software.
31+
#
32+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
33+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
34+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
35+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
36+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
37+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
38+
# SOFTWARE.
39+
40+
import unittest
41+
42+
43+
def test_starred_expr_with_iterable():
44+
class MyIterable:
45+
def __iter__(self):
46+
return [1,2,3].__iter__()
47+
assert [1,2,*MyIterable()] == [1,2,1,2,3]
48+
assert (1,2,*MyIterable()) == (1,2,1,2,3)
49+
assert {1,2,*MyIterable()} == {1,2,3}
50+
51+
52+
def test_starred_expr_with_call():
53+
def foo():
54+
return [1,2,3]
55+
56+
assert [1,2,*foo()] == [1,2,1,2,3]
57+
assert (1,2,*foo()) == (1,2,1,2,3)
58+
assert {1,2,*foo()} == {1,2,3}
59+
60+
61+
def test_starred_expr_with_literal():
62+
assert [1,2,*[1,2,3]] == [1,2,1,2,3]
63+
assert (1,2,*[1,2,3]) == (1,2,1,2,3)
64+
assert {1,2,*[1,2,3]} == {1,2,3}
65+
66+
67+
def test_complex_starred_expr():
68+
def bar():
69+
return ['a', 'b']
70+
71+
assert [1,2,*[1,[2,3]]] == [1,2,1,[2,3]]
72+
assert [1,2,*[1],*[2,3],*bar()] == [1,2,1,2,3,'a','b']
73+
74+
75+
class TestStarredExprErrors(unittest.TestCase):
76+
def test_starred_expr_no_iterable(self):
77+
self.assertRaises(TypeError, lambda : [1, *None])
78+
self.assertRaises(TypeError, lambda : [1, *1])

‎graalpython/com.oracle.graal.python.test/testData/goldenFiles/DictAndSetTests/set05.tast

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,15 @@ ModuleRootNode Name: <module 'set05'> SourceSection: [0,15]`{*{2}, 3, *[4]}`
77
InnerRootNode SourceSection: [0,15]`{*{2}, 3, *[4]}`
88
SetLiteralNodeGen SourceSection: [0,15]`{*{2}, 3, *[4]}`
99
PythonObjectFactoryNodeGen SourceSection: None
10-
StarredExpressionNode SourceSection: None
11-
CastToListExpressionNodeGen SourceSection: None
12-
SetLiteralNodeGen SourceSection: [2,5]`{2}`
13-
PythonObjectFactoryNodeGen SourceSection: None
14-
IntegerLiteralNode SourceSection: [3,4]`2`
15-
Value: 2
10+
StarredExpressionNode SourceSection: [1,5]`*{2}`
11+
SetLiteralNodeGen SourceSection: [2,5]`{2}`
12+
PythonObjectFactoryNodeGen SourceSection: None
13+
IntegerLiteralNode SourceSection: [3,4]`2`
14+
Value: 2
1615
IntegerLiteralNode SourceSection: [7,8]`3`
1716
Value: 3
18-
StarredExpressionNode SourceSection: None
19-
CastToListExpressionNodeGen SourceSection: None
20-
ListLiteralNode SourceSection: [11,14]`[4]`
21-
IntegerLiteralNode SourceSection: [12,13]`4`
22-
Value: 4
23-
PythonObjectFactoryNodeGen SourceSection: None
17+
StarredExpressionNode SourceSection: [10,14]`*[4]`
18+
ListLiteralNode SourceSection: [11,14]`[4]`
19+
IntegerLiteralNode SourceSection: [12,13]`4`
20+
Value: 4
21+
PythonObjectFactoryNodeGen SourceSection: None

‎graalpython/com.oracle.graal.python.test/testData/goldenFiles/ListAndSlicingTests/list05.tast

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,16 @@ ModuleRootNode Name: <module 'list05'> SourceSection: [0,15]`[*{2}, 3, *[4]]`
66
Documentation: None
77
InnerRootNode SourceSection: [0,15]`[*{2}, 3, *[4]]`
88
ListLiteralNode SourceSection: [0,15]`[*{2}, 3, *[4]]`
9-
StarredExpressionNode SourceSection: None
10-
CastToListExpressionNodeGen SourceSection: None
11-
SetLiteralNodeGen SourceSection: [2,5]`{2}`
12-
PythonObjectFactoryNodeGen SourceSection: None
13-
IntegerLiteralNode SourceSection: [3,4]`2`
14-
Value: 2
9+
StarredExpressionNode SourceSection: [1,5]`*{2}`
10+
SetLiteralNodeGen SourceSection: [2,5]`{2}`
11+
PythonObjectFactoryNodeGen SourceSection: None
12+
IntegerLiteralNode SourceSection: [3,4]`2`
13+
Value: 2
1514
IntegerLiteralNode SourceSection: [7,8]`3`
1615
Value: 3
17-
StarredExpressionNode SourceSection: None
18-
CastToListExpressionNodeGen SourceSection: None
19-
ListLiteralNode SourceSection: [11,14]`[4]`
20-
IntegerLiteralNode SourceSection: [12,13]`4`
21-
Value: 4
22-
PythonObjectFactoryNodeGen SourceSection: None
16+
StarredExpressionNode SourceSection: [10,14]`*[4]`
17+
ListLiteralNode SourceSection: [11,14]`[4]`
18+
IntegerLiteralNode SourceSection: [12,13]`4`
19+
Value: 4
20+
PythonObjectFactoryNodeGen SourceSection: None
2321
PythonObjectFactoryNodeGen SourceSection: None

‎graalpython/com.oracle.graal.python.test/testData/goldenFiles/ListAndSlicingTests/starExpr01.tast

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@ ModuleRootNode Name: <module 'starExpr01'> SourceSection: [0,10]`[*[1,2,3]]`
66
Documentation: None
77
InnerRootNode SourceSection: [0,10]`[*[1,2,3]]`
88
ListLiteralNode SourceSection: [0,10]`[*[1,2,3]]`
9-
StarredExpressionNode SourceSection: None
10-
CastToListExpressionNodeGen SourceSection: None
11-
ListLiteralNode SourceSection: [2,9]`[1,2,3]`
12-
IntegerLiteralNode SourceSection: [3,4]`1`
13-
Value: 1
14-
IntegerLiteralNode SourceSection: [5,6]`2`
15-
Value: 2
16-
IntegerLiteralNode SourceSection: [7,8]`3`
17-
Value: 3
18-
PythonObjectFactoryNodeGen SourceSection: None
9+
StarredExpressionNode SourceSection: [1,9]`*[1,2,3]`
10+
ListLiteralNode SourceSection: [2,9]`[1,2,3]`
11+
IntegerLiteralNode SourceSection: [3,4]`1`
12+
Value: 1
13+
IntegerLiteralNode SourceSection: [5,6]`2`
14+
Value: 2
15+
IntegerLiteralNode SourceSection: [7,8]`3`
16+
Value: 3
17+
PythonObjectFactoryNodeGen SourceSection: None
1918
PythonObjectFactoryNodeGen SourceSection: None

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public HashingStorage setItemWithState(HashingStorage self, Object key, Object v
151151
/**
152152
* @see #setItemWithState(HashingStorage, Object, Object, ThreadState)
153153
*/
154-
public HashingStorage setItem(HashingStorage self, Object key, Object value) {
154+
public final HashingStorage setItem(HashingStorage self, Object key, Object value) {
155155
return setItemWithState(self, key, value, null);
156156
}
157157

@@ -351,6 +351,7 @@ public static final class HashingStorageIterable<T> implements Iterable<T> {
351351
this.iterator = iterator;
352352
}
353353

354+
@Override
354355
public HashingStorageIterator<T> iterator() {
355356
return new HashingStorageIterator<T>(iterator);
356357
}
@@ -366,11 +367,13 @@ public HashingStorageIterator(Iterator<T> iterator) {
366367
this.iterator = iterator;
367368
}
368369

370+
@Override
369371
@TruffleBoundary
370372
public boolean hasNext() {
371373
return iterator.hasNext();
372374
}
373375

376+
@Override
374377
@TruffleBoundary
375378
public T next() {
376379
return iterator.next();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PArguments.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates.
33
* Copyright (c) 2013, Regents of the University of California
44
*
55
* All rights reserved.
@@ -38,6 +38,7 @@
3838
import com.oracle.truffle.api.frame.FrameDescriptor;
3939
import com.oracle.truffle.api.frame.MaterializedFrame;
4040
import com.oracle.truffle.api.frame.VirtualFrame;
41+
import com.oracle.truffle.api.profiles.ConditionProfile;
4142

4243
//@formatter:off
4344
/**
@@ -356,6 +357,10 @@ public static ThreadState getThreadState(VirtualFrame frame) {
356357
return new ThreadState(PArguments.getCurrentFrameInfo(frame), PArguments.getException(frame));
357358
}
358359

360+
public static ThreadState getThreadStateOrNull(VirtualFrame frame, ConditionProfile hasFrameProfile) {
361+
return hasFrameProfile.profile(frame != null) ? getThreadState(frame) : null;
362+
}
363+
359364
public static VirtualFrame frameForCall(ThreadState frame) {
360365
Object[] args = PArguments.create();
361366
PArguments.setCurrentFrameInfo(args, frame.info);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/expression/ExpressionNode.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -102,6 +102,11 @@ public boolean hasSideEffectAsAnExpression() {
102102
return false;
103103
}
104104

105+
public ExpressionNode unwrap() {
106+
return this instanceof ExpressionNodeWrapper ? ((ExpressionNodeWrapper) this).getDelegateNode() : this;
107+
}
108+
109+
@Override
105110
public WrapperNode createWrapper(ProbeNode probe) {
106111
return new ExpressionNodeWrapper(this, probe);
107112
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/literal/ListLiteralNode.java

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ public final class ListLiteralNode extends SequenceLiteralNode {
4747
private static final TruffleLogger LOGGER = PythonLanguage.getLogger(ListLiteralNode.class);
4848

4949
@Child private PythonObjectFactory factory = PythonObjectFactory.create();
50-
@Child private SequenceStorageNodes.ConcatNode concatStoragesNode;
5150
@Child private SequenceStorageNodes.AppendNode appendNode;
5251

5352
/**
@@ -96,11 +95,10 @@ private PList expandingList(VirtualFrame frame) {
9695
// we will usually have more than 'values.length' elements
9796
SequenceStorage storage = new ObjectSequenceStorage(values.length);
9897
for (ExpressionNode n : values) {
99-
if (n instanceof StarredExpressionNode) {
100-
SequenceStorage addElements = ((StarredExpressionNode) n).getStorage(frame);
101-
storage = ensureConcatStoragesNode().execute(storage, addElements);
98+
Object element = n.execute(frame);
99+
if (StarredExpressionNode.isStarredExpression(n)) {
100+
storage = ((StarredExpressionNode) n.unwrap()).appendToStorage(frame, storage, element);
102101
} else {
103-
Object element = n.execute(frame);
104102
storage = ensureAppendNode().execute(storage, element, ListGeneralizationNode.SUPPLIER);
105103
}
106104
}
@@ -126,14 +124,6 @@ protected int getCapacityEstimate() {
126124
return initialCapacity.estimate();
127125
}
128126

129-
private SequenceStorageNodes.ConcatNode ensureConcatStoragesNode() {
130-
if (concatStoragesNode == null) {
131-
CompilerDirectives.transferToInterpreterAndInvalidate();
132-
concatStoragesNode = insert(SequenceStorageNodes.ConcatNode.create(ListGeneralizationNode::create));
133-
}
134-
return concatStoragesNode;
135-
}
136-
137127
private SequenceStorageNodes.AppendNode ensureAppendNode() {
138128
if (appendNode == null) {
139129
CompilerDirectives.transferToInterpreterAndInvalidate();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/literal/SetLiteralNode.java

Lines changed: 5 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,12 @@
2828
import com.oracle.graal.python.builtins.objects.PNone;
2929
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
3030
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
31-
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
3231
import com.oracle.graal.python.builtins.objects.dict.PDict;
3332
import com.oracle.graal.python.builtins.objects.function.PArguments;
3433
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
3534
import com.oracle.graal.python.builtins.objects.set.PSet;
36-
import com.oracle.graal.python.nodes.PNode;
3735
import com.oracle.graal.python.nodes.expression.ExpressionNode;
3836
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
39-
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
40-
import com.oracle.truffle.api.CompilerDirectives;
4137
import com.oracle.truffle.api.dsl.Cached;
4238
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
4339
import com.oracle.truffle.api.dsl.Specialization;
@@ -50,98 +46,27 @@
5046
public abstract class SetLiteralNode extends LiteralNode {
5147
@Child private PythonObjectFactory factory = PythonObjectFactory.create();
5248
@Children private final ExpressionNode[] values;
53-
@Child private SequenceStorageNodes.LenNode lenNode;
54-
@Child private SequenceStorageNodes.GetItemNode getItemNode;
55-
56-
private final boolean hasStarredExpressions;
5749

5850
protected SetLiteralNode(ExpressionNode[] values) {
5951
this.values = values;
60-
for (PNode v : values) {
61-
if (v instanceof StarredExpressionNode) {
62-
hasStarredExpressions = true;
63-
return;
64-
}
65-
}
66-
hasStarredExpressions = false;
6752
}
6853

6954
@Specialization
55+
@ExplodeLoop
7056
public PSet expand(VirtualFrame frame,
7157
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
7258
@CachedLibrary(limit = "3") HashingStorageLibrary lib) {
73-
if (!hasStarredExpressions) {
74-
return directSet(frame, hasFrame, lib);
75-
} else {
76-
return expandingSet(frame, hasFrame, lib);
77-
}
78-
}
79-
80-
@ExplodeLoop
81-
private PSet expandingSet(VirtualFrame frame, ConditionProfile hasFrame, HashingStorageLibrary lib) {
8259
// we will usually have more than 'values.length' elements
8360
HashingStorage storage = PDict.createNewStorage(true, values.length);
84-
ThreadState state = PArguments.getThreadState(frame);
61+
ThreadState state = PArguments.getThreadStateOrNull(frame, hasFrame);
8562
for (ExpressionNode n : values) {
86-
if (n instanceof StarredExpressionNode) {
87-
storage = addAllElement(frame, storage, ((StarredExpressionNode) n).getStorage(frame), hasFrame, lib);
63+
Object element = n.execute(frame);
64+
if (StarredExpressionNode.isStarredExpression(n)) {
65+
storage = ((StarredExpressionNode) n.unwrap()).appendToSet(frame, storage, lib, state, element);
8866
} else {
89-
Object element = n.execute(frame);
90-
if (hasFrame.profile(frame != null)) {
91-
storage = lib.setItemWithState(storage, element, PNone.NONE, state);
92-
} else {
93-
storage = lib.setItem(storage, element, PNone.NONE);
94-
}
95-
}
96-
}
97-
return factory.createSet(storage);
98-
}
99-
100-
private HashingStorage addAllElement(VirtualFrame frame, HashingStorage setStorage, SequenceStorage sequenceStorage,
101-
ConditionProfile hasFrame, HashingStorageLibrary lib) {
102-
HashingStorage storage = setStorage;
103-
ThreadState state = PArguments.getThreadState(frame);
104-
int n = ensureLenNode().execute(sequenceStorage);
105-
for (int i = 0; i < n; i++) {
106-
Object element = ensureGetItemNode().execute(frame, sequenceStorage, i);
107-
if (hasFrame.profile(frame != null)) {
10867
storage = lib.setItemWithState(storage, element, PNone.NONE, state);
109-
} else {
110-
storage = lib.setItem(storage, element, PNone.NONE);
111-
}
112-
}
113-
return storage;
114-
}
115-
116-
@ExplodeLoop
117-
private PSet directSet(VirtualFrame frame, ConditionProfile hasFrame, HashingStorageLibrary lib) {
118-
HashingStorage storage = PDict.createNewStorage(true, values.length);
119-
ThreadState state = PArguments.getThreadState(frame);
120-
for (ExpressionNode v : this.values) {
121-
Object element = v.execute(frame);
122-
if (hasFrame.profile(frame != null)) {
123-
storage = lib.setItemWithState(storage, element, PNone.NONE, state);
124-
} else {
125-
storage = lib.setItem(storage, element, PNone.NONE);
12668
}
12769
}
12870
return factory.createSet(storage);
12971
}
130-
131-
private SequenceStorageNodes.LenNode ensureLenNode() {
132-
if (lenNode == null) {
133-
CompilerDirectives.transferToInterpreterAndInvalidate();
134-
lenNode = insert(SequenceStorageNodes.LenNode.create());
135-
}
136-
return lenNode;
137-
}
138-
139-
private SequenceStorageNodes.GetItemNode ensureGetItemNode() {
140-
if (getItemNode == null) {
141-
CompilerDirectives.transferToInterpreterAndInvalidate();
142-
getItemNode = insert(SequenceStorageNodes.GetItemNode.create());
143-
}
144-
return getItemNode;
145-
}
146-
14772
}

0 commit comments

Comments
 (0)