Skip to content

Commit e2bb9aa

Browse files
committed
[GR-13658] Introduce format specifier for exception messages.
PullRequest: graalpython/458
2 parents e0414e3 + c658620 commit e2bb9aa

File tree

11 files changed

+52
-33
lines changed

11 files changed

+52
-33
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ private Object loadDynamicModuleWithSpec(String name, String path, InteropLibrar
179179
CallTarget callTarget = env.parse(Source.newBuilder(LLVM_LANGUAGE, env.getTruffleFile(path)).build());
180180
sulongLibrary = (TruffleObject) callTarget.call();
181181
} catch (SecurityException | IOException e) {
182-
throw raise(ImportError, "cannot load %s: %s", path, e.getMessage());
182+
throw raise(ImportError, "cannot load %s: %m", path, e);
183183
} catch (RuntimeException e) {
184184
throw reportImportError(e, path);
185185
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ Object evalString(@SuppressWarnings("unused") PNone path, String value, String l
143143
}
144144
return env.parse(newBuilder.build()).call();
145145
} catch (RuntimeException e) {
146-
throw raise(NotImplementedError, e.getMessage());
146+
throw raise(NotImplementedError, e);
147147
}
148148
}
149149

@@ -170,7 +170,7 @@ Object evalFile(String path, @SuppressWarnings("unused") PNone string, String la
170170
} catch (IOException e) {
171171
throw raise(OSError, "%s", e);
172172
} catch (RuntimeException e) {
173-
throw raise(NotImplementedError, e.getMessage());
173+
throw raise(NotImplementedError, e);
174174
}
175175
}
176176

@@ -183,7 +183,7 @@ Object evalFile(String path, @SuppressWarnings("unused") PNone string, @Suppress
183183
} catch (IOException e) {
184184
throw raise(OSError, "%s", e);
185185
} catch (RuntimeException e) {
186-
throw raise(NotImplementedError, e.getMessage());
186+
throw raise(NotImplementedError, e);
187187
}
188188
}
189189

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ private String decode(byte[] raw) {
757757
try {
758758
return new String(raw, "ascii");
759759
} catch (UnsupportedEncodingException e) {
760-
throw raise(PythonBuiltinClassType.UnicodeDecodeError, e.getMessage());
760+
throw raise(PythonBuiltinClassType.UnicodeDecodeError, e);
761761
}
762762
}
763763

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ synchronized int forkExec(PList args, @SuppressWarnings("unused") PList execList
157157
throw raise(PythonBuiltinClassType.OSError, "working directory %s is not accessible", cwd);
158158
}
159159
} catch (SecurityException e) {
160-
throw raise(PythonBuiltinClassType.OSError, e.getMessage());
160+
throw raise(PythonBuiltinClassType.OSError, e);
161161
}
162162

163163
Map<String, String> environment = pb.environment();

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

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -737,17 +737,12 @@ Object doIt(VirtualFrame frame,
737737
context.setCurrentException(exceptionState);
738738
return result;
739739
} catch (UnsupportedTypeException | UnsupportedMessageException e) {
740-
throw context.getCore().raise(PythonBuiltinClassType.TypeError, "Calling native function %s failed: %s", name, getMessage(e));
740+
throw context.getCore().raise(PythonBuiltinClassType.TypeError, "Calling native function %s failed: %m", name, e);
741741
} catch (ArityException e) {
742742
throw context.getCore().raise(PythonBuiltinClassType.TypeError, "Calling native function %s expected %d arguments but got %d.", name, e.getExpectedArity(), e.getActualArity());
743743
}
744744
}
745745

746-
@TruffleBoundary
747-
private static final String getMessage(Throwable e) {
748-
return e.getMessage();
749-
}
750-
751746
private Object fromNative(Object result) {
752747
return fromForeign.executeConvert(result);
753748
}
@@ -833,11 +828,6 @@ protected static ByteBuffer wrap(byte[] data) {
833828
protected static ByteBuffer wrap(byte[] data, int offset, int length) {
834829
return ByteBuffer.wrap(data, offset, length);
835830
}
836-
837-
@TruffleBoundary
838-
protected static String getMessage(Throwable throwable) {
839-
return throwable.getMessage();
840-
}
841831
}
842832

843833
abstract static class NativeUnicodeBuiltin extends NativeBuiltin {
@@ -1007,11 +997,11 @@ Object doBytes(TruffleObject o, long elementSize, Object errorMarker,
1007997
}
1008998
return decoded.toString();
1009999
} catch (CharacterCodingException e) {
1010-
return raiseNative(errorMarker, PythonErrorType.UnicodeError, e.getMessage());
1000+
return raiseNative(errorMarker, PythonErrorType.UnicodeError, "%m", e);
10111001
} catch (IllegalArgumentException e) {
1012-
return raiseNative(errorMarker, PythonErrorType.LookupError, e.getMessage());
1002+
return raiseNative(errorMarker, PythonErrorType.LookupError, "%m", e);
10131003
} catch (InteropException e) {
1014-
return raiseNative(errorMarker, PythonErrorType.TypeError, e.getMessage());
1004+
return raiseNative(errorMarker, PythonErrorType.TypeError, "%m", e);
10151005
}
10161006
}
10171007

@@ -1040,9 +1030,9 @@ Object doBytes(TruffleObject o, Object errorMarker,
10401030
CharBuffer cbuf = decoder.decode(wrap(getByteArrayNode.execute(o, -1)));
10411031
return cbuf.toString();
10421032
} catch (CharacterCodingException e) {
1043-
return raiseNative(errorMarker, PythonErrorType.UnicodeError, e.getMessage());
1033+
return raiseNative(errorMarker, PythonErrorType.UnicodeError, "%m", e);
10441034
} catch (InteropException e) {
1045-
return raiseNative(errorMarker, PythonErrorType.TypeError, e.getMessage());
1035+
return raiseNative(errorMarker, PythonErrorType.TypeError, "%m", e);
10461036
}
10471037
}
10481038
}
@@ -1077,7 +1067,7 @@ Object doUnicode(PString s, String errors, Object error_marker) {
10771067
transformToNative(e);
10781068
return error_marker;
10791069
} catch (CharacterCodingException e) {
1080-
return raiseNative(error_marker, PythonErrorType.UnicodeEncodeError, e.getMessage());
1070+
return raiseNative(error_marker, PythonErrorType.UnicodeEncodeError, "%m", e);
10811071
}
10821072
}
10831073

@@ -1150,12 +1140,12 @@ Object doUnicode(TruffleObject o, long size, String errors, int byteorder, Objec
11501140
CharBuffer decode = decoder.onMalformedInput(action).onUnmappableCharacter(action).decode(wrap(getByteArrayNode.execute(o, size), 0, (int) size));
11511141
return toSulongNode.execute(decode.toString());
11521142
} catch (CharacterCodingException e) {
1153-
return raiseNative(errorMarker, PythonErrorType.UnicodeEncodeError, e.getMessage());
1143+
return raiseNative(errorMarker, PythonErrorType.UnicodeEncodeError, "%m", e);
11541144
} catch (IllegalArgumentException e) {
11551145
String csName = getUTF32Name(byteorder);
11561146
return raiseNative(errorMarker, PythonErrorType.LookupError, "unknown encoding: " + csName);
11571147
} catch (InteropException e) {
1158-
return raiseNative(errorMarker, PythonErrorType.TypeError, e.getMessage());
1148+
return raiseNative(errorMarker, PythonErrorType.TypeError, "%m", e);
11591149
}
11601150
}
11611151
}
@@ -1229,7 +1219,7 @@ Object doUnicode(String s, long elementSize, long elements, Object errorMarker)
12291219
}
12301220
} catch (IllegalArgumentException e) {
12311221
// TODO
1232-
return raiseNative(errorMarker, PythonErrorType.LookupError, e.getMessage());
1222+
return raiseNative(errorMarker, PythonErrorType.LookupError, "%m", e);
12331223
}
12341224
}
12351225

@@ -2156,7 +2146,7 @@ Object doGeneric(Object module, PythonNativeObject object,
21562146
try {
21572147
return factory().createBytes(getByteArrayNode.execute(object.getPtr(), -1));
21582148
} catch (InteropException e) {
2159-
return raiseNative(getNativeNullNode.execute(module), PythonErrorType.TypeError, getMessage(e));
2149+
return raiseNative(getNativeNullNode.execute(module), PythonErrorType.TypeError, "%m", e);
21602150
}
21612151
}
21622152
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ Object decompress(InflaterWrapper stream, PIBytesLike pb, int maxLen) {
501501
try {
502502
bytesWritten = stream.inflater.inflate(result, 0, result.length);
503503
} catch (DataFormatException e) {
504-
throw raise(ZLibError, e.getMessage());
504+
throw raise(ZLibError, e);
505505
}
506506
baos.write(result, 0, bytesWritten);
507507
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/BaseExceptionBuiltins.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ private GetLazyClassNode getGetClassNode() {
109109
@TruffleBoundary
110110
private String getFormattedMessage(String format, Object... args) {
111111
try {
112-
// pre-format for '%p' which retrieves the Python class of an argument
113-
if (format.contains("%p")) {
112+
// pre-format for custom error message formatter
113+
if (ErrorMessageFormatter.containsCustomSpecifier(format)) {
114114
return formatter.format(getGetClassNode(), format, args);
115115
}
116116
return String.format(format, args);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ public static PythonAbstractClass[] doSlowPath(Object obj) {
546546
try {
547547
return cast(SequenceStorageNodes.ToArrayNode.doSlowPath(basesTuple.getSequenceStorage()));
548548
} catch (ClassCastException e) {
549-
throw PythonLanguage.getCore().raise(PythonBuiltinClassType.SystemError, "unsupported object in 'tp_bases' (msg: %s)", e.getMessage());
549+
throw PythonLanguage.getCore().raise(PythonBuiltinClassType.SystemError, "unsupported object in 'tp_bases' (msg: %m)", e);
550550
}
551551
}
552552
throw new IllegalStateException("unknown type " + obj.getClass().getName());

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/statement/TryExceptNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private boolean shouldCatchAll() {
148148

149149
@TruffleBoundary
150150
private PBaseException getBaseException(Exception t) {
151-
return factory().createBaseException(PythonErrorType.ValueError, t.getMessage(), new Object[0]);
151+
return factory().createBaseException(PythonErrorType.ValueError, "%m", new Object[]{t});
152152
}
153153

154154
@ExplodeLoop

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/formatting/ErrorMessageFormatter.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646

4747
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
4848
import com.oracle.graal.python.nodes.object.GetLazyClassNode;
49+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4950

5051
/**
5152
* Custom formatter adding Python-specific conversions often required in error messages.
@@ -94,6 +95,14 @@ public String format(GetLazyClassNode getClassNode, String format, Object... arg
9495
offset += name.length() - (m.end() - m.start());
9596
args[matchIdx] = REMOVED_MARKER;
9697
removedCnt++;
98+
} else if ("%m".equals(group) && args[matchIdx] instanceof Throwable) {
99+
// If the format arg is not a Throwable, 'String.format' will do the error handling
100+
// and throw an IllegalFormatException for us.
101+
String exceptionMessage = getMessage((Throwable) args[matchIdx]);
102+
sb.replace(m.start() + offset, m.end() + offset, exceptionMessage);
103+
offset += exceptionMessage.length() - (m.end() - m.start());
104+
args[matchIdx] = REMOVED_MARKER;
105+
removedCnt++;
97106
}
98107

99108
idx = m.end();
@@ -105,13 +114,33 @@ public String format(GetLazyClassNode getClassNode, String format, Object... arg
105114
return String.format(sb.toString(), compact(args, removedCnt));
106115
}
107116

117+
@TruffleBoundary
118+
private static String getMessage(Throwable exception) {
119+
return exception.getMessage();
120+
}
121+
108122
private static String getClassName(GetLazyClassNode getClassNode, Object obj) {
109123
if (getClassNode != null) {
110124
return GetNameNode.doSlowPath(getClassNode.execute(obj));
111125
}
112126
return GetNameNode.doSlowPath(GetLazyClassNode.getUncached().execute(obj));
113127
}
114128

129+
/**
130+
* Use this method to check if a given format string contains any of the custom format
131+
* specifiers handled by this formatter.
132+
*/
133+
public static boolean containsCustomSpecifier(String format) {
134+
int pidx = -1;
135+
while ((pidx = format.indexOf('%', pidx + 1)) != -1 && pidx + 1 < format.length()) {
136+
char c = format.charAt(pidx + 1);
137+
if (c == 'p' || c == 'P' || c == 'm') {
138+
return true;
139+
}
140+
}
141+
return false;
142+
}
143+
115144
private static Object[] compact(Object[] args, int removedCnt) {
116145
Object[] compacted = new Object[args.length - removedCnt];
117146
int j = 0;

0 commit comments

Comments
 (0)