Skip to content

Commit

Permalink
Adress reviewer comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
chumer committed Jul 21, 2021
1 parent de5a700 commit b7ac9d0
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 37 deletions.
2 changes: 1 addition & 1 deletion truffle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ This changelog summarizes major changes between Truffle versions relevant to lan
* Added `TruffleContext.Builder.initializeCreatorContext(boolean)` that allows to disable initialization of the language that created the inner context.

## Version 21.3.0
* Added the ability to share values between contexts. Guest languages can now use values of the polyglot embedding API using host interop. This no no longer leads to invalid sharing errors.
* Added the ability to share values between contexts. Guest languages can now use values of the polyglot embedding API using host interop. This no longer leads to invalid sharing errors.
* Added `ReflectionLibrary.getUncached` methods.

## Version 21.2.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ public DebugValue convertRawValue(Class<? extends TruffleLanguage<?>> languageCl
return null;
}
// make sure rawValue is a valid Interop value
if (!Debugger.ACCESSOR.interopSupport().isInteropType(rawValue)) {
if (!InteropLibrary.isValidValue(rawValue)) {
throw new IllegalArgumentException("raw value is not an Interop value");
}
// check if language class of the root node corresponds to the input language
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ static class EvalContextTestException extends AbstractTruffleException {
TruffleContext expectedContext;
int executeCount = 0;

@SuppressWarnings("static-method")
@ExportMessage
@TruffleBoundary
final boolean isException() {
Expand Down Expand Up @@ -564,7 +563,7 @@ public void testEvalInnerContext() throws InteropException {
Object result = innerContext.eval(null, newTruffleSource());
assertEquals(42, result);

// test that objects that cross the boundary enterd in the inner context
// test that objects that cross the boundary are entered in the inner context
EvalContextTestObject innerObject = new EvalContextTestObject();
EvalContextTestObject outerObject = new EvalContextTestObject();
setupLanguageThatReturns(() -> innerObject);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private static String violationArgument(Object receiver, Object arg) {
}

static boolean validInteropReturn(Object receiver, Object arg) {
assert isInteropValue(arg) : violationReturn(receiver, arg);
assert InteropLibrary.isValidValue(arg) : violationReturn(receiver, arg);
return true;
}

Expand Down Expand Up @@ -176,11 +176,6 @@ private static String violationNonInteropArgument(Object receiver, Object arg) {
formatValue(receiver), formatValue(arg));
}

@SuppressWarnings("deprecation")
static boolean isInteropValue(Object o) {
return InteropLibrary.isValidValue(o);
}

static boolean validArguments(Object receiver, Object[] args) {
assert args != null : violationPre(receiver);
for (Object arg : args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ static Object checkInteropType(Object obj) {
}

private static boolean checkInteropTypeImpl(Object obj) {
if (AssertUtils.isInteropValue(obj)) {
if (InteropLibrary.isValidValue(obj)) {
return true;
}
CompilerDirectives.transferToInterpreterAndInvalidate();
Expand Down Expand Up @@ -98,11 +98,6 @@ public void checkInteropType(Object result) {
InteropAccessor.checkInteropType(result);
}

@Override
public boolean isInteropType(Object result) {
return AssertUtils.isInteropValue(result);
}

@Override
public boolean isExecutableObject(Object value) {
return InteropLibrary.getUncached().isExecutable(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@
package com.oracle.truffle.api.interop;

import static com.oracle.truffle.api.interop.AssertUtils.assertString;
import static com.oracle.truffle.api.interop.AssertUtils.isInteropValue;
import static com.oracle.truffle.api.interop.AssertUtils.validScope;
import static com.oracle.truffle.api.interop.AssertUtils.violationInvariant;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.TruffleLanguage;
import com.oracle.truffle.api.frame.Frame;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.library.GenerateLibrary;
import com.oracle.truffle.api.library.GenerateLibrary.Abstract;
import com.oracle.truffle.api.library.GenerateLibrary.DefaultExport;
Expand Down Expand Up @@ -425,7 +425,7 @@ public Object getRootInstance(Object node, Frame frame) throws UnsupportedMessag
}

private static void assertValidRootInstance(Object instance) {
assert isInteropValue(instance) : violationInvariant(instance);
assert InteropLibrary.isValidValue(instance) : violationInvariant(instance);
assert InteropLibrary.getUncached().isExecutable(instance) : String.format("The root instance '%s' is not executable.", instance);
}

Expand All @@ -435,10 +435,10 @@ public Object getView(Object node, Frame frame, Object value) {
return delegate.getView(node, frame, value);
}
assert validReceiver(node);
assert isInteropValue(value) : violationInvariant(value);
assert InteropLibrary.isValidValue(value) : violationInvariant(value);
Class<?> languageClass = validateLocationAndFrame((Node) node, frame, value);
Object view = delegate.getView(node, frame, value);
assert isInteropValue(view) : violationInvariant(view);
assert InteropLibrary.isValidValue(view) : violationInvariant(view);
InteropLibrary lib = InteropLibrary.getUncached(view);
try {
assert lib.hasLanguage(view) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ protected InteropSupport() {

public abstract void checkInteropType(Object result);

public abstract boolean isInteropType(Object result);

public abstract boolean isExecutableObject(Object value);

public abstract Object createDefaultNodeObject(Node node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static Object getVariables(RootNode root, Frame frame, Class<? extends TruffleLa
int lastI = 0;
for (int i = 0; i < slots.size(); i++) {
FrameSlot slot = slots.get(i);
if (!EngineAccessor.INTEROP.isInteropType(frame.getValue(slot)) || isInternal(slot)) {
if (!InteropLibrary.isValidValue(frame.getValue(slot)) || isInternal(slot)) {
if (nonNulls == null) {
nonNulls = new ArrayList<>(slots.size());
}
Expand Down Expand Up @@ -343,7 +343,7 @@ boolean isMemberReadable(String member) {
try {
int index = Integer.parseInt(member);
if (0 <= index && index < args.length) {
return EngineAccessor.INTEROP.isInteropType(args[index]);
return InteropLibrary.isValidValue(args[index]);
} else {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,12 @@ static Object doSlowPath(OtherContextGuestObject receiver, Message message, Obje

private static final Message IDENTICAL = Message.resolve(InteropLibrary.class, "isIdentical");

private static Object sendImpl(PolyglotEngineImpl engine, Object receiver, Message message, Object[] args, PolyglotContextImpl receiverContext,
static Object sendImpl(PolyglotEngineImpl engine, Object receiver, Message message, Object[] args, PolyglotContextImpl receiverContext,
PolyglotContextImpl delegateContext,
ReflectionLibrary delegateLibrary,
BranchProfile seenOther,
BranchProfile seenError) throws Exception {
if (message.getLibraryClass() == InteropLibrary.class) {
assert validEnteredContext(receiverContext);

PolyglotContextImpl prev;
try {
prev = engine.enter(delegateContext);
Expand Down Expand Up @@ -196,14 +194,6 @@ static <T extends Exception> RuntimeException migrateException(PolyglotContextIm
}
}

private static boolean validEnteredContext(PolyglotContextImpl receiverContext) {
PolyglotContextImpl currentContext = PolyglotContextImpl.currentNotEntered();
if (currentContext != null && currentContext != receiverContext) {
throw new AssertionError("A foreign context value was used while an invalid parent context was entered.");
}
return true;
}

private static RuntimeException toHostException(PolyglotContextImpl context, Throwable e) {
try {
throw PolyglotImpl.engineToLanguageException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ void resume(Future<Void> pauseFuture) {
}

/**
* Use to enter context if its guaranteed to be called rarely and configuration flexibility is
* Use to enter context if it's guaranteed to be called rarely and configuration flexibility is
* needed. Otherwise use {@link PolyglotEngineImpl#enter(PolyglotContextImpl)}.
*/
@TruffleBoundary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,7 @@ private PolyglotContextImpl enterBoundary(PolyglotContextImpl context) {
}

/**
* Only use to leave contexts from paths that are *always* compiled otherwise use
* Only use to enter contexts from paths that are *always* compiled otherwise use
* {@link #enter(PolyglotContextImpl)}.
*/
PolyglotContextImpl enterCached(PolyglotContextImpl context, boolean pollSafepoint) {
Expand Down

0 comments on commit b7ac9d0

Please sign in to comment.