From e7c9a51f4ca236f477f24730ee386e91e29cde73 Mon Sep 17 00:00:00 2001 From: Lukas Stadler <lukas.stadler@oracle.com> Date: Thu, 22 Dec 2016 18:36:39 +0100 Subject: [PATCH] =?UTF-8?q?check=20for=20boolean=20frame=20slots=20in=20?= =?UTF-8?q?=E2=80=9CexecuteEndOfFunction=E2=80=9D=20and=20produce=20proper?= =?UTF-8?q?=20error=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../com/oracle/truffle/r/engine/REngine.java | 7 ++++++- .../com/oracle/truffle/r/nodes/RRootNode.java | 5 +++++ .../r/nodes/builtin/RBuiltinRootNode.java | 7 +------ .../nodes/function/FunctionDefinitionNode.java | 7 +------ .../function/visibility/SetVisibilityNode.java | 18 ++++++++++++++---- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java b/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java index ee090e1c1a..971bbf6d7b 100644 --- a/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java +++ b/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java @@ -526,7 +526,7 @@ final class REngine implements Engine, Engine.Timings { if (topLevel) { RErrorHandling.printWarnings(suppressWarnings); } - setVisibility.executeEndOfFunction(vf); + setVisibility.executeEndOfFunction(vf, this); } catch (RError e) { CompilerDirectives.transferToInterpreter(); throw e; @@ -558,6 +558,11 @@ final class REngine implements Engine, Engine.Timings { return result; } + @Override + public String getName() { + return description; + } + @Override public String toString() { return description; diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RRootNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RRootNode.java index 78ecc50564..929e539386 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RRootNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/RRootNode.java @@ -105,4 +105,9 @@ public abstract class RRootNode extends RootNode implements HasSignature { public boolean isCloningAllowed() { return true; } + + @Override + public final String toString() { + return getName(); + } } diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/RBuiltinRootNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/RBuiltinRootNode.java index 1754b1a1f9..9e0044e241 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/RBuiltinRootNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/RBuiltinRootNode.java @@ -89,7 +89,7 @@ public final class RBuiltinRootNode extends RRootNode { throw new RInternalError(e, "internal error"); } finally { visibility.execute(frame, factory.getVisibility()); - visibility.executeEndOfFunction(frame); + visibility.executeEndOfFunction(frame, this); } } @@ -121,9 +121,4 @@ public final class RBuiltinRootNode extends RRootNode { public String getName() { return "RBuiltin(" + builtin + ")"; } - - @Override - public String toString() { - return getName(); - } } diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java index 5e212366e4..4744ee9b83 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java @@ -278,7 +278,7 @@ public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNo * has no exit handlers (by fiat), so any exceptions from onExits handlers will be * caught above. */ - visibility.executeEndOfFunction(frame); + visibility.executeEndOfFunction(frame, this); if (argPostProcess != null) { resetArgs.enter(); argPostProcess.execute(frame); @@ -406,11 +406,6 @@ public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNo return name == null ? "<no source>" : name; } - @Override - public String toString() { - return getName(); - } - public void setName(String name) { this.name = name; } diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/visibility/SetVisibilityNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/visibility/SetVisibilityNode.java index c4946fcc9e..e5315d7410 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/visibility/SetVisibilityNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/visibility/SetVisibilityNode.java @@ -22,6 +22,7 @@ */ package com.oracle.truffle.r.nodes.function.visibility; +import com.oracle.truffle.api.CompilerAsserts; import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; import com.oracle.truffle.api.frame.Frame; @@ -32,6 +33,7 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.NodeCost; import com.oracle.truffle.api.nodes.NodeInfo; +import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.r.runtime.RArguments; import com.oracle.truffle.r.runtime.RCaller; import com.oracle.truffle.r.runtime.RInternalError; @@ -86,12 +88,19 @@ public final class SetVisibilityNode extends Node { * Needs to be called at the end of each function, so that the visibility is transferred from * the current frame into the {@link RCaller}. */ - public void executeEndOfFunction(VirtualFrame frame) { + public void executeEndOfFunction(VirtualFrame frame, RootNode root) { ensureFrameSlot(frame); try { - Object visibility = frame.getBoolean(frameSlot); - if (visibility != null) { - RArguments.getCall(frame).setVisibility(visibility == Boolean.TRUE); + if (frame.isBoolean(frameSlot)) { + RArguments.getCall(frame).setVisibility(frame.getBoolean(frameSlot) == Boolean.TRUE); + } else { + CompilerDirectives.transferToInterpreter(); + /* + * Most likely the (only) builtin call in the function was configured to + * RVisibility.CUSTOM and didn't actually set the visibility. Another possible + * problem is a node that is created by RASTBuilder that does not set visibility. + */ + throw RInternalError.shouldNotReachHere("visibility not set at the end of " + root.getName()); } } catch (FrameSlotTypeException e) { throw RInternalError.shouldNotReachHere(e); @@ -102,6 +111,7 @@ public final class SetVisibilityNode extends Node { * Slow-path version of {@link #executeAfterCall(VirtualFrame, RCaller)}. */ public static void executeAfterCallSlowPath(Frame frame, RCaller caller) { + CompilerAsserts.neverPartOfCompilation(); frame.setBoolean(frame.getFrameDescriptor().findOrAddFrameSlot(RFrameSlot.Visibility, FrameSlotKind.Boolean), caller.getVisibility()); } } -- GitLab