Skip to content

Commit 74fbbfc

Browse files
phoeinxphoeinx
committed
Refactor according to PR review.
Refactorings include: - Module builtin functions now declare explicit self / take module as $self parameter. - Move all reader / parser and writer code to methods defined on CSVReader and CSVWriter and behind a truffle boundary - Add PComplex specializations for PyComplexCheck(Exact)Node and PString specialization for PyUnicodeCheckNode and PyObjectStrAsJavaStringNode
1 parent a6e903b commit 74fbbfc

File tree

11 files changed

+156
-132
lines changed

11 files changed

+156
-132
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/csv/CSVModuleBuiltins.java

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,18 @@ public void initialize(Python3Core core) {
7171
builtinConstants.put("QUOTE_ALL", QUOTE_ALL.ordinal());
7272
builtinConstants.put("QUOTE_NONNUMERIC", QUOTE_NONNUMERIC.ordinal());
7373
builtinConstants.put("QUOTE_NONE", QUOTE_NONE.ordinal());
74-
builtinConstants.put("_dialects", PythonObjectFactory.getUncached().createDict());
74+
builtinConstants.put("_dialects", core.factory().createDict());
7575
super.initialize(core);
7676
}
7777

78-
@Builtin(name = "register_dialect", parameterNames = {"name",
79-
"dialect"}, minNumOfPositionalArgs = 1, takesVarKeywordArgs = true, doc = "Create a mapping from a string name to a dialect class.\n" +
78+
@Builtin(name = "register_dialect", parameterNames = { "$mod","name",
79+
"dialect"}, minNumOfPositionalArgs = 2, takesVarKeywordArgs = true, declaresExplicitSelf = true, doc = "Create a mapping from a string name to a dialect class.\n" +
8080
"dialect = csv.register_dialect(name, dialect)")
8181
@GenerateNodeFactory
8282
public abstract static class CSVRegisterDialectNode extends PythonBuiltinNode {
8383

8484
@Specialization
85-
PNone register(VirtualFrame frame, Object nameObj, Object dialectObj, PKeyword[] keywords,
85+
PNone register(VirtualFrame frame, PythonModule module, Object nameObj, Object dialectObj, PKeyword[] keywords,
8686
@Cached CastToJavaStringNode nameNode,
8787
@Cached ReadAttributeFromObjectNode readNode,
8888
@Cached CallNode callNode,
@@ -97,9 +97,7 @@ PNone register(VirtualFrame frame, Object nameObj, Object dialectObj, PKeyword[]
9797

9898
Object result = callNode.execute(frame, PythonBuiltinClassType.CSVDialect, new Object[]{dialectObj}, keywords);
9999

100-
PythonModule module = getCore().lookupBuiltinModule("_csv");
101100
Object dialects = readNode.execute(module, "_dialects");
102-
103101
// TODO: Write PyDictSetItem Node?
104102
HashingStorage storage = library.setItem(((PDict) dialects).getDictStorage(), name, result);
105103
((PDict) dialects).setDictStorage(storage);
@@ -108,16 +106,15 @@ PNone register(VirtualFrame frame, Object nameObj, Object dialectObj, PKeyword[]
108106
}
109107
}
110108

111-
@Builtin(name = "unregister_dialect", parameterNames = {"name"}, minNumOfPositionalArgs = 1, doc = "Delete the name/dialect mapping associated with a string name.\n" +
109+
@Builtin(name = "unregister_dialect", parameterNames = {"$mod", "name"}, minNumOfPositionalArgs = 2, declaresExplicitSelf = true, doc = "Delete the name/dialect mapping associated with a string name.\n" +
112110
"csv.unregister_dialect(name)")
113111
@GenerateNodeFactory
114112
public abstract static class CSVUnregisterDialectNode extends PythonBuiltinNode {
115113
@Specialization
116-
PNone unregister(Object nameObj,
114+
PNone unregister(PythonModule module, Object nameObj,
117115
@Cached ReadAttributeFromObjectNode readNode,
118116
@CachedLibrary(limit = "1") HashingStorageLibrary library) {
119117

120-
PythonModule module = getCore().lookupBuiltinModule("_csv");
121118
Object dialects = readNode.execute(module, "_dialects");
122119

123120
//TODO: Should we write a PyDict_DelItem Node?
@@ -131,22 +128,21 @@ PNone unregister(Object nameObj,
131128
}
132129
}
133130

134-
@Builtin(name = "get_dialect", parameterNames = {"name"}, minNumOfPositionalArgs = 1, doc = "Return the dialect instance associated with name.\n" +
131+
@Builtin(name = "get_dialect", parameterNames = {"$mod", "name"}, minNumOfPositionalArgs = 2, declaresExplicitSelf = true, doc = "Return the dialect instance associated with name.\n" +
135132
"dialect = csv.get_dialect(name)")
136133
@GenerateNodeFactory
137134
public abstract static class CSVGetDialectNode extends PythonBuiltinNode {
138135

139-
public abstract CSVDialect execute(VirtualFrame frame, Object name);
136+
public abstract CSVDialect execute(VirtualFrame frame, PythonModule module, Object name);
140137

141138
protected static CSVGetDialectNode create() {
142139
return CSVModuleBuiltinsFactory.CSVGetDialectNodeFactory.create(null);
143140
}
144141
@Specialization
145-
CSVDialect get(VirtualFrame frame, Object nameObj,
142+
CSVDialect get(VirtualFrame frame, PythonModule module, Object nameObj,
146143
@Cached PyDictGetItem getItemNode,
147144
@Cached ReadAttributeFromObjectNode readNode) {
148145

149-
PythonModule module = getCore().lookupBuiltinModule("_csv");
150146
PDict dialects = (PDict) readNode.execute(module, "_dialects");
151147

152148
CSVDialect dialect = (CSVDialect) getItemNode.execute(frame, dialects, nameObj);
@@ -159,19 +155,17 @@ CSVDialect get(VirtualFrame frame, Object nameObj,
159155
}
160156
}
161157

162-
@Builtin(name = "list_dialects", doc = "Return a list of all known dialect names.\n" +
158+
@Builtin(name = "list_dialects", parameterNames = {"$mod"}, declaresExplicitSelf = true, doc = "Return a list of all known dialect names.\n" +
163159
"names = csv.list_dialects()")
164160
@GenerateNodeFactory
165161
public abstract static class CSVListDialectsNode extends PythonBuiltinNode {
166162
@Specialization
167-
PList listDialects(VirtualFrame frame,
163+
PList listDialects(VirtualFrame frame, PythonModule module,
168164
@Cached ReadAttributeFromObjectNode readNode,
169165
@CachedLibrary(limit = "1") HashingStorageLibrary library,
170166
@Cached ListNodes.ConstructListNode constructListNode) {
171167

172-
PythonModule module = getCore().lookupBuiltinModule("_csv");
173168
Object dialects = readNode.execute(module, "_dialects");
174-
175169
return constructListNode.execute(frame, dialects);
176170
}
177171
}
@@ -234,13 +228,13 @@ Object createReader(VirtualFrame frame, Object outputFile, Object dialectObj, PK
234228
public abstract static class CSVFieldSizeLimitNode extends PythonBuiltinNode {
235229
@Specialization
236230
long getOrSetFieldSizeLimit(VirtualFrame frame, Object newLimit,
237-
@Cached PyLongCheckExactNode checkIntNode,
231+
@Cached PyLongCheckExactNode checkLongNode,
238232
@Cached PyLongAsLongNode castToLong) {
239233

240234
long oldLimit = fieldLimit;
241235

242236
if (newLimit != PNone.NO_VALUE) {
243-
if (!PyLongCheckExactNode.getUncached().execute(newLimit)) {
237+
if (!checkLongNode.execute(newLimit)) {
244238
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.MUST_BE_INTEGER, "limit");
245239
}
246240

@@ -271,7 +265,8 @@ CSVDialect doStringWithoutKeywords(VirtualFrame frame, PythonBuiltinClassType cl
271265
@Cached CSVModuleBuiltins.CSVGetDialectNode getDialect,
272266
@Cached ReadAttributeFromObjectNode readNode,
273267
@Cached PyDictGetItem getItemNode) {
274-
return getDialect.get(frame, dialectName, getItemNode, readNode);
268+
PythonModule module = getCore().lookupBuiltinModule("_csv");
269+
return getDialect.get(frame, module, dialectName, getItemNode, readNode);
275270
}
276271

277272
@Specialization
@@ -299,8 +294,8 @@ Object doStringWithKeywords(VirtualFrame frame, PythonBuiltinClassType cls, Stri
299294
@Cached PyObjectIsTrueNode isTrueNode,
300295
@Cached PyLongCheckExactNode pyLongCheckExactNode,
301296
@Cached PyLongAsIntNode pyLongAsIntNode) {
302-
303-
CSVDialect dialectObj = getDialect.get(frame, dialectName, getItemNode, readNode);
297+
PythonModule module = getCore().lookupBuiltinModule("_csv");
298+
CSVDialect dialectObj = getDialect.get(frame, module, dialectName, getItemNode, readNode);
304299

305300
if (delimiterObj == PNone.NO_VALUE) delimiterObj = dialectObj.delimiter;
306301
if (doublequoteObj == PNone.NO_VALUE) doublequoteObj = dialectObj.doubleQuote;
@@ -355,7 +350,8 @@ Object doPStringWithKeywords(VirtualFrame frame, PythonBuiltinClassType cls, PSt
355350
@Cached PyLongAsIntNode pyLongAsIntNode) {
356351

357352
String dialectNameStr = castToJavaStringNode.execute(dialectName);
358-
CSVDialect dialectObj = getDialect.get(frame, dialectNameStr, getItemNode, readNode);
353+
PythonModule module = getCore().lookupBuiltinModule("_csv");
354+
CSVDialect dialectObj = getDialect.get(frame, module, dialectNameStr, getItemNode, readNode);
359355

360356
if (delimiterObj == PNone.NO_VALUE) delimiterObj = dialectObj.delimiter;
361357
if (doublequoteObj == PNone.NO_VALUE) doublequoteObj = dialectObj.doubleQuote;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/csv/CSVReader.java

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@
55
import com.oracle.graal.python.lib.PyNumberFloatNode;
66
import com.oracle.graal.python.nodes.ErrorMessages;
77
import com.oracle.graal.python.nodes.PRaiseNode;
8+
import com.oracle.graal.python.nodes.control.GetNextNode;
9+
import com.oracle.graal.python.nodes.object.GetClassNode;
10+
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
11+
import com.oracle.graal.python.nodes.util.CannotCastException;
12+
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
13+
import com.oracle.graal.python.runtime.exception.PException;
14+
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
15+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
16+
import com.oracle.truffle.api.exception.AbstractTruffleException;
817
import com.oracle.truffle.api.object.Shape;
918

1019
import java.util.ArrayList;
@@ -27,7 +36,7 @@ public final class CSVReader extends PythonBuiltinObject {
2736
private static final int CARRIAGE_RETURN_CODEPOINT = "\r".codePointAt(0);
2837
private static final int SPACE_CODEPOINT = " ".codePointAt(0);
2938

30-
public enum ReaderState {
39+
enum ReaderState {
3140
START_RECORD,
3241
START_FIELD,
3342
ESCAPED_CHAR,
@@ -58,7 +67,7 @@ void parseReset() {
5867
this.state = START_RECORD;
5968
this.numericField = false;
6069
}
61-
70+
6271
void parseSaveField() {
6372
Object field = this.field.toString();
6473
this.field = new StringBuilder();
@@ -71,6 +80,57 @@ void parseSaveField() {
7180
this.fields.add(field);
7281
}
7382

83+
@TruffleBoundary
84+
Object parseIterableInput() {
85+
do {
86+
Object lineObj;
87+
try {
88+
lineObj = GetNextNode.getUncached().execute(this.inputIter);
89+
} catch (PException e) {
90+
e.expectStopIteration(IsBuiltinClassProfile.getUncached());
91+
92+
if (this.field.length() != 0 || this.state == IN_QUOTED_FIELD) {
93+
if (this.dialect.strict) {
94+
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.UNEXPECTED_END_OF_DATA);
95+
} else {
96+
try {
97+
this.parseSaveField();
98+
} catch (AbstractTruffleException ignored) {
99+
throw e.getExceptionForReraise();
100+
}
101+
break;
102+
}
103+
}
104+
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.StopIteration);
105+
}
106+
107+
108+
String line;
109+
try {
110+
line = CastToJavaStringNode.getUncached().execute(lineObj);
111+
} catch (CannotCastException e) {
112+
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.WRONG_ITERATOR_RETURN_TYPE, GetClassNode.getUncached().execute(lineObj));
113+
}
114+
115+
116+
//TODO: Implement PyUnicode_Check Node? => how do we handle the possibility of bytes?
117+
// PyPy: if isinstance(line, str) and '\0' in line or isinstance(line, bytes) and line.index(0) >=0:
118+
// raise Error("line contains NULL byte")
119+
if (line.contains("\u0000")) {
120+
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.LINE_CONTAINS_NULL_BYTE);
121+
}
122+
123+
this.lineNum++;
124+
this.parseLine(line);
125+
126+
} while (this.state != START_RECORD);
127+
128+
ArrayList<Object> fields = this.fields;
129+
this.fields = null;
130+
131+
return PythonObjectFactory.getUncached().createList(fields.toArray());
132+
}
133+
74134
void parseLine(String line) {
75135
final int lineLength = line.length();
76136

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/csv/CSVReaderBuiltins.java

Lines changed: 2 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,16 @@
66
import com.oracle.graal.python.builtins.CoreFunctions;
77
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
88
import com.oracle.graal.python.builtins.PythonBuiltins;
9-
import com.oracle.graal.python.nodes.ErrorMessages;
10-
import com.oracle.graal.python.nodes.control.GetNextNode;
119
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
1210
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
13-
import com.oracle.graal.python.nodes.object.GetClassNode;
14-
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
15-
import com.oracle.graal.python.nodes.util.CannotCastException;
16-
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
1711
import com.oracle.graal.python.runtime.ExecutionContext.IndirectCallContext;
18-
import com.oracle.graal.python.runtime.exception.PException;
19-
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
20-
import com.oracle.truffle.api.dsl.Cached;
2112
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
2213
import com.oracle.truffle.api.dsl.NodeFactory;
2314
import com.oracle.truffle.api.dsl.Specialization;
24-
import com.oracle.truffle.api.exception.AbstractTruffleException;
2515
import com.oracle.truffle.api.frame.VirtualFrame;
2616

27-
import java.util.ArrayList;
2817
import java.util.List;
2918

30-
import static com.oracle.graal.python.builtins.modules.csv.CSVReader.ReaderState.IN_QUOTED_FIELD;
31-
import static com.oracle.graal.python.builtins.modules.csv.CSVReader.ReaderState.START_RECORD;
3219
import static com.oracle.graal.python.nodes.SpecialMethodNames.__ITER__;
3320
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEXT__;
3421

@@ -53,67 +40,15 @@ Object iter(CSVReader self) {
5340
@GenerateNodeFactory
5441
public abstract static class NextReaderNode extends PythonUnaryBuiltinNode {
5542
@Specialization
56-
Object nextPos(VirtualFrame frame, CSVReader self,
57-
@Cached GetNextNode nextNode,
58-
@Cached CastToJavaStringNode castToJavaStringNode,
59-
@Cached PythonObjectFactory factory,
60-
@Cached GetClassNode getClassNode) {
43+
Object nextPos(VirtualFrame frame, CSVReader self) {
6144

6245
self.parseReset();
6346

6447
PythonLanguage language = PythonLanguage.get(this);
6548
Object state = IndirectCallContext.enter(frame, language, getContext(), this);
6649

6750
try {
68-
69-
do {
70-
Object lineObj;
71-
try {
72-
lineObj = nextNode.execute(frame, self.inputIter);
73-
} catch (PException e) {
74-
e.expectStopIteration(IsBuiltinClassProfile.getUncached());
75-
76-
if ( self.field.length() != 0 || self.state == IN_QUOTED_FIELD) {
77-
if (self.dialect.strict) {
78-
throw raise(PythonBuiltinClassType.CSVError, ErrorMessages.UNEXPECTED_END_OF_DATA);
79-
} else {
80-
try {
81-
self.parseSaveField();
82-
} catch (AbstractTruffleException ignored) {
83-
throw e.getExceptionForReraise();
84-
}
85-
break;
86-
}
87-
}
88-
throw raise(PythonBuiltinClassType.StopIteration);
89-
}
90-
91-
92-
String line;
93-
try {
94-
line = castToJavaStringNode.execute(lineObj);
95-
} catch (CannotCastException e) {
96-
throw raise(PythonBuiltinClassType.CSVError, ErrorMessages.WRONG_ITERATOR_RETURN_TYPE, getClassNode.execute(lineObj));
97-
}
98-
99-
100-
//TODO: Implement PyUnicode_Check Node? => how do we handle the possibility of bytes?
101-
// PyPy: if isinstance(line, str) and '\0' in line or isinstance(line, bytes) and line.index(0) >=0:
102-
// raise Error("line contains NULL byte")
103-
if (line.contains("\u0000")) {
104-
throw raise(PythonBuiltinClassType.CSVError, ErrorMessages.LINE_CONTAINS_NULL_BYTE);
105-
}
106-
107-
self.lineNum++;
108-
self.parseLine(line);
109-
110-
} while (self.state != START_RECORD);
111-
112-
ArrayList<Object> fields = self.fields;
113-
self.fields = null;
114-
115-
return factory.createList(fields.toArray());
116-
51+
return self.parseIterableInput();
11752
} finally {
11853
IndirectCallContext.exit(frame, language, getContext(), state);
11954
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/csv/CSVWriter.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
import com.oracle.graal.python.lib.PyObjectStrAsJavaStringNode;
88
import com.oracle.graal.python.nodes.ErrorMessages;
99
import com.oracle.graal.python.nodes.PRaiseNode;
10+
import com.oracle.graal.python.nodes.control.GetNextNode;
11+
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
12+
import com.oracle.graal.python.runtime.exception.PException;
13+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
1014
import com.oracle.truffle.api.object.Shape;
1115

1216
import static com.oracle.graal.python.builtins.modules.csv.CSVModuleBuiltins.NOT_SET;
@@ -29,6 +33,33 @@ void joinReset() {
2933
this.numFields = 0;
3034
}
3135

36+
@TruffleBoundary
37+
void joinFields(Object iter) {
38+
Object field;
39+
40+
this.joinReset();
41+
42+
while (true) {
43+
try {
44+
field = GetNextNode.getUncached().execute(iter);
45+
this.joinField(field);
46+
} catch (PException e) {
47+
e.expectStopIteration(IsBuiltinClassProfile.getUncached());
48+
break;
49+
}
50+
}
51+
52+
if (this.numFields > 0 && this.rec.length() == 0) {
53+
if (this.dialect.quoting == QUOTE_NONE) {
54+
throw PRaiseNode.getUncached().raise(PythonBuiltinClassType.CSVError, ErrorMessages.EMPTY_FIELD_RECORD_MUST_BE_QUOTED);
55+
}
56+
this.numFields--;
57+
this.joinAppend(null, true);
58+
}
59+
60+
this.joinAppendLineterminator();
61+
}
62+
3263
void joinField(Object field) {
3364
boolean quoted;
3465

0 commit comments

Comments
 (0)