Skip to content

Commit 5278b4b

Browse files
msimacekmsimacek
committed
[GR-54789] Refactor polyglot.eval to use new form of isPolyglotEvalAllowed
PullRequest: graalpython/3373
2 parents 6c47bdd + 0ed7e20 commit 5278b4b

File tree

9 files changed

+64
-376
lines changed

9 files changed

+64
-376
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ language runtime. The main focus is on user-observable behavior of the engine.
66
## Version 24.1.0
77
* Update to Python 3.11.7
88
* We now provide intrinsified `_pickle` module also in the community version.
9+
* `polyglot.eval` now raises more meaningful exceptions. Unavaliable languages raise `ValueError`. Exceptions from the polyglot language are raised directly as interop objects (typed as `polyglot.ForeignException`). The shortcut for executing python files without specifying language has been removed, use regular `eval` for executing Python code.
910

1011
## Version 24.0.0
1112
* We now provide a collection of recipes in the form of GitHub Actions to build popular native extensions on GraalPy. These provide a reproducible way for the community to build native extensions for GraalPy with the correct dependencies. See scripts/wheelbuilder/README.md for details.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ def test_host_lookup():
286286
def test_internal_languages_dont_eval():
287287
try:
288288
polyglot.eval(language="nfi", string="default")
289-
except NotImplementedError as e:
290-
assert "No language for id nfi found" in str(e)
289+
except ValueError as e:
290+
assert str(e) == "polyglot language 'nfi' not found"
291291

292292
assert polyglot.eval(language="python", string="21 * 2") == 42
293293

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,9 @@ def run_serialize_out(cmd):
129129
result = run_with_timeout(cmd, timeout=RUN_TIMEOUT, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
130130
append_out = "-v" in sys.argv
131131
if result.returncode == 124:
132-
raise subprocess.TimeoutExpired(cmd=cmd, timeout=RUN_TIMEOUT)
133-
if result.returncode:
132+
message = f"{working_test[0]} timed out after {RUN_TIMEOUT}s.\n"
133+
append_out = True
134+
elif result.returncode:
134135
message = f"{working_test[0]} failed with exit code {result.returncode}.\n"
135136
append_out = True
136137
else:
@@ -142,6 +143,8 @@ def run_serialize_out(cmd):
142143
message += LINE + "\n"
143144
print(message)
144145
if result.returncode:
146+
if result.returncode == 124:
147+
raise subprocess.TimeoutExpired(cmd=cmd, timeout=RUN_TIMEOUT)
145148
raise subprocess.CalledProcessError(result.returncode, cmd)
146149

147150
if testmod.startswith('test_'):

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

Lines changed: 0 additions & 287 deletions
Large diffs are not rendered by default.

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ public enum PythonBuiltinClassType implements TruffleObject {
494494
SSLSyscallError("SSLSyscallError", J__SSL, Flags.EXCEPTION),
495495
SSLEOFError("SSLEOFError", J__SSL, Flags.EXCEPTION),
496496
SSLCertVerificationError("SSLCertVerificationError", J__SSL, Flags.EXCEPTION),
497-
PForeignException("ForeignException", Flags.FOREIGN_EXCEPTION),
497+
PForeignException("ForeignException", J_POLYGLOT, Flags.FOREIGN_EXCEPTION),
498498

499499
// todo: all OS errors
500500

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

Lines changed: 48 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@
6161
import static com.oracle.graal.python.nodes.StringLiterals.T_READABLE;
6262
import static com.oracle.graal.python.nodes.StringLiterals.T_WRITABLE;
6363
import static com.oracle.graal.python.nodes.truffle.TruffleStringMigrationHelpers.isJavaString;
64-
import static com.oracle.graal.python.runtime.exception.PythonErrorType.NotImplementedError;
6564
import static com.oracle.graal.python.runtime.exception.PythonErrorType.OSError;
65+
import static com.oracle.graal.python.runtime.exception.PythonErrorType.RuntimeError;
66+
import static com.oracle.graal.python.runtime.exception.PythonErrorType.TypeError;
6667
import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
6768
import static com.oracle.graal.python.util.PythonUtils.EMPTY_BYTE_ARRAY;
6869
import static com.oracle.graal.python.util.PythonUtils.EMPTY_OBJECT_ARRAY;
@@ -111,6 +112,7 @@
111112
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
112113
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
113114
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
115+
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
114116
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
115117
import com.oracle.graal.python.nodes.interop.InteropBehavior;
116118
import com.oracle.graal.python.nodes.interop.InteropBehaviorMethod;
@@ -138,11 +140,11 @@
138140
import com.oracle.truffle.api.TruffleLogger;
139141
import com.oracle.truffle.api.dsl.Bind;
140142
import com.oracle.truffle.api.dsl.Cached;
141-
import com.oracle.truffle.api.dsl.Cached.Shared;
142143
import com.oracle.truffle.api.dsl.Fallback;
143144
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
144145
import com.oracle.truffle.api.dsl.Specialization;
145146
import com.oracle.truffle.api.dsl.TypeSystemReference;
147+
import com.oracle.truffle.api.exception.AbstractTruffleException;
146148
import com.oracle.truffle.api.frame.VirtualFrame;
147149
import com.oracle.truffle.api.interop.ArityException;
148150
import com.oracle.truffle.api.interop.InteropLibrary;
@@ -154,7 +156,6 @@
154156
import com.oracle.truffle.api.nodes.LanguageInfo;
155157
import com.oracle.truffle.api.nodes.Node;
156158
import com.oracle.truffle.api.source.Source;
157-
import com.oracle.truffle.api.source.Source.LiteralBuilder;
158159
import com.oracle.truffle.api.source.Source.SourceBuilder;
159160
import com.oracle.truffle.api.strings.TruffleString;
160161

@@ -213,99 +214,68 @@ Object importSymbol(TruffleString name,
213214

214215
@Builtin(name = "eval", minNumOfPositionalArgs = 0, parameterNames = {"path", "string", "language"})
215216
@GenerateNodeFactory
216-
abstract static class EvalInteropNode extends PythonBuiltinNode {
217+
abstract static class EvalInteropNode extends PythonTernaryBuiltinNode {
217218
@TruffleBoundary
218219
@Specialization
219-
Object evalString(@SuppressWarnings("unused") PNone path, TruffleString tvalue, TruffleString tlangOrMimeType,
220-
@Shared @Cached PForeignToPTypeNode convert) {
220+
Object eval(Object pathObj, Object stringObj, Object languageObj) {
221+
if (languageObj instanceof PNone) {
222+
throw PRaiseNode.raiseUncached(this, ValueError, ErrorMessages.POLYGLOT_EVAL_MUST_PASS_LANG);
223+
}
224+
boolean hasString = !(stringObj instanceof PNone);
225+
boolean hasPath = !(pathObj instanceof PNone);
226+
if (!hasString && !hasPath || hasString && hasPath) {
227+
throw PRaiseNode.raiseUncached(this, ValueError, ErrorMessages.POLYGLOT_EVAL_MUST_PASS_STRING_OR_PATH);
228+
}
229+
String languageName = toJavaString(languageObj, "language");
230+
String string = hasString ? toJavaString(stringObj, "string") : null;
231+
String path = hasPath ? toJavaString(pathObj, "path") : null;
221232
Env env = getContext().getEnv();
222-
if (!env.isPolyglotEvalAllowed()) {
223-
throw PRaiseNode.raiseUncached(this, PythonErrorType.NotImplementedError, ErrorMessages.POLYGLOT_ACCESS_NOT_ALLOWED);
233+
if (!env.isPolyglotEvalAllowed(null)) {
234+
throw PRaiseNode.raiseUncached(this, RuntimeError, ErrorMessages.POLYGLOT_ACCESS_NOT_ALLOWED);
224235
}
225-
try {
226-
String value = tvalue.toJavaStringUncached();
227-
String langOrMimeType = tlangOrMimeType.toJavaStringUncached();
228-
boolean mimeType = isMimeType(langOrMimeType);
229-
String lang = mimeType ? findLanguageByMimeType(env, langOrMimeType) : langOrMimeType;
230-
raiseIfInternal(env, lang);
231-
LiteralBuilder newBuilder = Source.newBuilder(lang, value, value);
232-
if (mimeType) {
233-
newBuilder = newBuilder.mimeType(langOrMimeType);
234-
}
235-
return convert.executeConvert(env.parsePublic(newBuilder.build()).call());
236-
} catch (RuntimeException e) {
237-
throw PRaiseNode.raiseUncached(this, NotImplementedError, e);
236+
Map<String, LanguageInfo> languages = env.getPublicLanguages();
237+
String mimeType = null;
238+
if (isMimeType(languageName)) {
239+
mimeType = languageName;
240+
languageName = findLanguageByMimeType(languages, languageName);
238241
}
239-
}
240-
241-
private void raiseIfInternal(Env env, String lang) {
242-
LanguageInfo languageInfo = env.getPublicLanguages().get(lang);
243-
if (languageInfo != null && languageInfo.isInternal()) {
244-
throw PRaiseNode.raiseUncached(this, NotImplementedError, ErrorMessages.ACCESS_TO_INTERNAL_LANG_NOT_PERMITTED, lang);
242+
LanguageInfo language = languages.get(languageName);
243+
if (language == null) {
244+
throw PRaiseNode.raiseUncached(this, ValueError, ErrorMessages.POLYGLOT_LANGUAGE_S_NOT_FOUND, languageName);
245245
}
246-
}
247-
248-
@TruffleBoundary
249-
@Specialization
250-
Object evalFile(TruffleString tpath, @SuppressWarnings("unused") PNone string, TruffleString tlangOrMimeType,
251-
@Shared @Cached PForeignToPTypeNode convert) {
252-
Env env = getContext().getEnv();
253-
if (!env.isPolyglotEvalAllowed()) {
254-
throw PRaiseNode.raiseUncached(this, PythonErrorType.NotImplementedError, ErrorMessages.POLYGLOT_ACCESS_NOT_ALLOWED);
246+
if (!env.isPolyglotEvalAllowed(language)) {
247+
throw PRaiseNode.raiseUncached(this, RuntimeError, ErrorMessages.POLYGLOT_ACCESS_NOT_ALLOWED_FOR_LANGUAGE_S, languageName);
255248
}
256249
try {
257-
String path = tpath.toJavaStringUncached();
258-
String langOrMimeType = tlangOrMimeType.toJavaStringUncached();
259-
boolean mimeType = isMimeType(langOrMimeType);
260-
String lang = mimeType ? findLanguageByMimeType(env, langOrMimeType) : langOrMimeType;
261-
raiseIfInternal(env, lang);
262-
SourceBuilder newBuilder = Source.newBuilder(lang, env.getPublicTruffleFile(path));
263-
if (mimeType) {
264-
newBuilder = newBuilder.mimeType(langOrMimeType);
250+
SourceBuilder builder;
251+
if (hasString) {
252+
builder = Source.newBuilder(languageName, string, string);
253+
} else {
254+
builder = Source.newBuilder(languageName, env.getPublicTruffleFile(path)).name(path);
255+
}
256+
if (mimeType != null) {
257+
builder = builder.mimeType(mimeType);
265258
}
266-
return convert.executeConvert(getContext().getEnv().parsePublic(newBuilder.name(path).build()).call());
259+
Object result = env.parsePublic(builder.build()).call();
260+
return PForeignToPTypeNode.getUncached().executeConvert(result);
261+
} catch (AbstractTruffleException e) {
262+
throw e;
267263
} catch (IOException e) {
268264
throw PRaiseNode.raiseUncached(this, OSError, ErrorMessages.S, e);
269265
} catch (RuntimeException e) {
270-
throw PRaiseNode.raiseUncached(this, NotImplementedError, e);
266+
throw PRaiseNode.raiseUncached(this, RuntimeError, e);
271267
}
272268
}
273269

274-
@TruffleBoundary
275-
@Specialization
276-
Object evalFile(TruffleString tpath, @SuppressWarnings("unused") PNone string, @SuppressWarnings("unused") PNone lang,
277-
@Shared @Cached PForeignToPTypeNode convert) {
278-
Env env = getContext().getEnv();
279-
if (!env.isPolyglotEvalAllowed()) {
280-
throw PRaiseNode.raiseUncached(this, PythonErrorType.NotImplementedError, ErrorMessages.POLYGLOT_ACCESS_NOT_ALLOWED);
281-
}
270+
private String toJavaString(Object object, String parameterName) {
282271
try {
283-
String path = tpath.toJavaStringUncached();
284-
return convert.executeConvert(getContext().getEnv().parsePublic(Source.newBuilder(PythonLanguage.ID, env.getPublicTruffleFile(path)).name(path).build()).call());
285-
} catch (IOException e) {
286-
throw PRaiseNode.raiseUncached(this, OSError, ErrorMessages.S, e);
287-
} catch (RuntimeException e) {
288-
throw PRaiseNode.raiseUncached(this, NotImplementedError, e);
272+
return CastToJavaStringNode.getUncached().execute(object);
273+
} catch (CannotCastException e) {
274+
throw PRaiseNode.raiseUncached(this, TypeError, ErrorMessages.S_BRACKETS_ARG_S_MUST_BE_S_NOT_P, "polyglot.eval", parameterName, "str", object);
289275
}
290276
}
291277

292-
@SuppressWarnings("unused")
293-
@Specialization
294-
static Object evalStringWithoutLang(PNone path, TruffleString string, PNone lang,
295-
@Shared @Cached PRaiseNode raiseNode) {
296-
throw raiseNode.raise(ValueError, ErrorMessages.POLYGLOT_EVAL_WITH_STRING_MUST_PASS_LANG);
297-
}
298-
299-
@SuppressWarnings("unused")
300-
@Fallback
301-
static Object evalWithoutContent(Object path, Object string, Object lang,
302-
@Shared @Cached PRaiseNode raiseNode) {
303-
throw raiseNode.raise(ValueError, ErrorMessages.POLYGLOT_EVAL_MUST_PASS_STRINGS);
304-
}
305-
306-
@TruffleBoundary(transferToInterpreterOnException = false)
307-
private static String findLanguageByMimeType(Env env, String mimeType) {
308-
Map<String, LanguageInfo> languages = env.getPublicLanguages();
278+
private static String findLanguageByMimeType(Map<String, LanguageInfo> languages, String mimeType) {
309279
for (String language : languages.keySet()) {
310280
for (String registeredMimeType : languages.get(language).getMimeTypes()) {
311281
if (mimeType.equals(registeredMimeType)) {
@@ -316,7 +286,7 @@ private static String findLanguageByMimeType(Env env, String mimeType) {
316286
return null;
317287
}
318288

319-
protected boolean isMimeType(String lang) {
289+
private static boolean isMimeType(String lang) {
320290
return lang.contains("/");
321291
}
322292
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,9 @@ public abstract class ErrorMessages {
586586
public static final TruffleString PACKED_IP_WRONG_LENGTH = tsLiteral("packed IP wrong length for %s");
587587
public static final TruffleString PATTERNS_MAY_ONLY_MATCH_LITERALS_AND_ATTRIBUTE_LOOKUPS = tsLiteral("patterns may only match literals and attribute lookups");
588588
public static final TruffleString POLYGLOT_ACCESS_NOT_ALLOWED = tsLiteral("polyglot access is not allowed");
589-
public static final TruffleString POLYGLOT_EVAL_MUST_PASS_STRINGS = tsLiteral("polyglot.eval must pass strings as either 'path' or a 'string' keyword");
590-
public static final TruffleString POLYGLOT_EVAL_WITH_STRING_MUST_PASS_LANG = tsLiteral("polyglot.eval with a string argument must pass a language or mime-type");
589+
public static final TruffleString POLYGLOT_ACCESS_NOT_ALLOWED_FOR_LANGUAGE_S = tsLiteral("polyglot access is not allowed for language '%s'");
590+
public static final TruffleString POLYGLOT_EVAL_MUST_PASS_STRING_OR_PATH = tsLiteral("polyglot.eval() must pass either 'path' or 'string' keyword argument");
591+
public static final TruffleString POLYGLOT_EVAL_MUST_PASS_LANG = tsLiteral("polyglot.eval() must pass 'language' keyword argument");
591592
public static final TruffleString POP_FROM_EMPTY_SET = tsLiteral("pop from an emtpy set");
592593
public static final TruffleString POP_INDEX_OUT_OF_RANGE = tsLiteral("pop index out of range");
593594
public static final TruffleString POSITION_D_FROM_ERROR_HANDLER_OUT_OF_BOUNDS = tsLiteral("position %d from error handler out of bounds");
@@ -782,7 +783,7 @@ public abstract class ErrorMessages {
782783
public static final TruffleString X_NOT_IN_TUPLE = tsLiteral("tuple.index(x): x not in tuple");
783784
public static final TruffleString S_IS_AN_INVALID_ARG_FOR_S = tsLiteral("'%s' is an invalid keyword argument for %s");
784785
public static final TruffleString YOU_MAY_SPECIFY_EITHER_OR_BUT_NOT_BOTH = tsLiteral("%s: you may specify either '%s' or '%s' but not both");
785-
public static final TruffleString ACCESS_TO_INTERNAL_LANG_NOT_PERMITTED = tsLiteral("access to internal language %s is not permitted");
786+
public static final TruffleString POLYGLOT_LANGUAGE_S_NOT_FOUND = tsLiteral("polyglot language '%s' not found");
786787
public static final TruffleString POW_BASE_NOT_INVERTIBLE = tsLiteral("base is not invertible for the given modulus");
787788
public static final TruffleString POW_ZERO_CANNOT_RAISE_TO_NEGATIVE_POWER = tsLiteral("0.0 cannot be raised to a negative power");
788789
public static final TruffleString S_ALIGNMENT_FLAG_NOT_ALLOWED_FOR_COMPLEX_FMT = tsLiteral("'%c' alignment flag is not allowed in complex format specifier");

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PRootNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2024, 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
@@ -128,7 +128,7 @@ public void setNeedsExceptionState() {
128128
}
129129

130130
@Override
131-
public boolean isCaptureFramesForTrace() {
131+
public boolean isCaptureFramesForTrace(Node node) {
132132
return true;
133133
}
134134

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/BuiltinFunctionRootNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ public boolean isCloningAllowed() {
317317
}
318318

319319
@Override
320-
public boolean isCaptureFramesForTrace() {
320+
public boolean isCaptureFramesForTrace(Node node) {
321321
return false;
322322
}
323323

0 commit comments

Comments
 (0)