Skip to content

Commit bc21e67

Browse files
steve-ssteve-s
committed
[GR-9993] [GR-8953] Fix parent.frame() after Rf_eval and various.
PullRequest: fastr/1531
2 parents c2b12f2 + ebde2bd commit bc21e67

File tree

15 files changed

+176
-55
lines changed

15 files changed

+176
-55
lines changed

‎ci_common/common.hocon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ common : ${java8Downloads} ${packagesLinux} {
8787
# the "mx" tool. This defines a common prefix for all gate commands. The "-t"
8888
# arg indicates the exact set of gate "tasks" that will be run.
8989

90-
gateCmd : ["mx", "-v", "--strict-compliance", "rgate", "-B=--force-deprecation-as-warning", "--strict-mode", "-t"]
90+
gateCmd : ["mx", "-v", "--strict-compliance", "rgate", "--strict-mode", "-t"]
9191

9292
# currently disabled gate commands: FindBugs,Checkheaders,Distribution Overlap Check
9393

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/RfEvalNode.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ public static RfEvalNode create() {
6363
private static RCaller createCall(REnvironment env) {
6464
Frame frame = Utils.getActualCurrentFrame();
6565
RCaller originalCaller = RArguments.getCall(env.getFrame());
66-
return RCaller.createForPromise(originalCaller, frame);
66+
if (env == REnvironment.globalEnv(RContext.getInstance())) {
67+
return RCaller.createForPromise(originalCaller, frame);
68+
} else {
69+
return RCaller.createForPromise(originalCaller, frame, env);
70+
}
6771
}
6872

6973
@Specialization

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java‎

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.function.Function;
3131

3232
import com.oracle.truffle.api.CompilerAsserts;
33+
import com.oracle.truffle.api.CompilerDirectives;
3334
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
3435
import com.oracle.truffle.api.dsl.Cached;
3536
import com.oracle.truffle.api.dsl.Specialization;
@@ -41,6 +42,7 @@
4142
import com.oracle.truffle.api.nodes.ExplodeLoop;
4243
import com.oracle.truffle.api.profiles.BranchProfile;
4344
import com.oracle.truffle.api.profiles.ConditionProfile;
45+
import com.oracle.truffle.api.profiles.ValueProfile;
4446
import com.oracle.truffle.r.nodes.RASTUtils;
4547
import com.oracle.truffle.r.nodes.RRootNode;
4648
import com.oracle.truffle.r.nodes.access.ConstantNode;
@@ -178,7 +180,7 @@ private static Frame getCallerFrame(Frame current) {
178180
closure.setNeedsCallerFrame();
179181
return closure.getMaterializedCallerFrame();
180182
}
181-
assert callerFrame instanceof Frame;
183+
assert callerFrame == null || callerFrame instanceof Frame;
182184
return (Frame) callerFrame;
183185
}
184186
}
@@ -609,10 +611,19 @@ public int[] apply(Frame f) {
609611
if (!currentCall.isPromise() && currentCall.getDepth() <= depth) {
610612
int currentCallIdx = currentCall.getDepth() - 1;
611613
RCaller parent = currentCall.getParent();
612-
while (parent != null && (parent.isPromise())) {
613-
parent = parent.getParent();
614+
int parentDepth;
615+
if (parent.hasSysParent()) {
616+
// parent.frame explicitly set by Rf_eval(quote(foo()), env) to env
617+
// GNU R gives parent number == frame created for calling foo().
618+
// There is no actual numbered frame that would be equal to env.
619+
parentDepth = currentCall.getDepth();
620+
} else {
621+
while (parent != null && (parent.isPromise())) {
622+
parent = parent.getParent();
623+
}
624+
parentDepth = parent == null ? 0 : parent.getDepth();
614625
}
615-
result[currentCallIdx] = parent == null ? 0 : parent.getDepth();
626+
result[currentCallIdx] = parentDepth;
616627
}
617628
return RArguments.getDepth(f) == 1 ? result : null;
618629
}
@@ -628,11 +639,8 @@ public int[] apply(Frame f) {
628639
@RBuiltin(name = "parent.frame", kind = SUBSTITUTE, parameterNames = {"n"}, behavior = COMPLEX)
629640
public abstract static class ParentFrame extends RBuiltinNode.Arg1 {
630641

631-
@Child private FrameHelper helper = new FrameHelper(FrameAccess.MATERIALIZE);
632-
633-
private final BranchProfile nullCallerProfile = BranchProfile.create();
634-
private final BranchProfile promiseProfile = BranchProfile.create();
635-
private final BranchProfile nonNullCallerProfile = BranchProfile.create();
642+
@Child private FrameHelper helper;
643+
@Child private GetCallerFrameNode getCaller;
636644

637645
public abstract REnvironment execute(VirtualFrame frame, int n);
638646

@@ -646,20 +654,20 @@ public Object[] getDefaultParameterValues() {
646654
return new Object[]{1};
647655
}
648656

649-
@Specialization(guards = "n == 1")
650-
protected REnvironment parentFrameDirect(VirtualFrame frame, @SuppressWarnings("unused") int n,
651-
@Cached("new()") GetCallerFrameNode getCaller) {
652-
// Note: this works even without checking the call#hasInternalParent()
653-
// The environment in the arguments array is the right one even after 'do.call'.
654-
return REnvironment.frameToEnvironment(getCaller.execute(frame));
655-
}
656-
657-
@Specialization(replaces = "parentFrameDirect")
658-
protected REnvironment parentFrame(VirtualFrame frame, int n) {
657+
@Specialization
658+
protected REnvironment parentFrame(VirtualFrame frame, int nIn,
659+
@Cached("create()") BranchProfile nullCallerProfile,
660+
@Cached("create()") BranchProfile promiseProfile,
661+
@Cached("create()") BranchProfile nonNullCallerProfile,
662+
@Cached("create()") BranchProfile explicitSysParent,
663+
@Cached("createEqualityProfile()") ValueProfile nProfile,
664+
@Cached("createBinaryProfile()") ConditionProfile useGetCallerFrameNode) {
665+
int n = nProfile.profile(nIn);
659666
if (n <= 0) {
660667
throw error(RError.Message.INVALID_VALUE, "n");
661668
}
662-
RCaller call = RArguments.getCall(frame);
669+
RCaller originalCall = RArguments.getCall(frame);
670+
RCaller call = originalCall;
663671
while (call.isPromise()) {
664672
promiseProfile.enter();
665673
call = call.getParent();
@@ -671,22 +679,40 @@ protected REnvironment parentFrame(VirtualFrame frame, int n) {
671679
nullCallerProfile.enter();
672680
return REnvironment.globalEnv();
673681
}
682+
if (i == n - 1 && call.hasSysParent()) {
683+
// promise RCallers are allowed to override "parent.frame"
684+
// this is necessary for Rf_eval(expr, env), where parent.frame() should be env
685+
explicitSysParent.enter();
686+
return call.getSysParent();
687+
}
674688
while (call.isPromise()) {
675689
promiseProfile.enter();
676690
call = call.getParent();
677691
}
678692
i++;
679693
}
680694
nonNullCallerProfile.enter();
681-
// if (RArguments.getDispatchArgs(f) != null && RArguments.getDispatchArgs(f) instanceof
682-
// S3Args) {
683-
// /*
684-
// * Skip the next frame if this frame has dispatch args, and therefore was
685-
// * called by UseMethod or NextMethod.
686-
// */
687-
// parentDepth--;
688-
// }
689-
return REnvironment.frameToEnvironment(helper.getNumberedFrame(frame, call.getDepth()).materialize());
695+
if (useGetCallerFrameNode.profile(originalCall.getDepth() == call.getDepth() + 1)) {
696+
// If the parent frame is the caller frame, we can use more efficient code:
697+
return REnvironment.frameToEnvironment(getCallerFrameNode().execute(frame));
698+
}
699+
return REnvironment.frameToEnvironment(getFrameHelper().getNumberedFrame(frame, call.getDepth()).materialize());
700+
}
701+
702+
private FrameHelper getFrameHelper() {
703+
if (helper == null) {
704+
CompilerDirectives.transferToInterpreterAndInvalidate();
705+
helper = insert(new FrameHelper(FrameAccess.MATERIALIZE));
706+
}
707+
return helper;
708+
}
709+
710+
private GetCallerFrameNode getCallerFrameNode() {
711+
if (getCaller == null) {
712+
CompilerDirectives.transferToInterpreterAndInvalidate();
713+
getCaller = insert(new GetCallerFrameNode());
714+
}
715+
return getCaller;
690716
}
691717
}
692718
}

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/IsFiniteFunctions.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ protected RLogicalVector doFunComplex(RAbstractComplexVector x, ComplexPredicate
146146
}
147147
}
148148

149-
@RBuiltin(name = "is.finite", kind = PRIMITIVE, parameterNames = {"x"}, behavior = PURE)
149+
@RBuiltin(name = "is.finite", kind = PRIMITIVE, dispatch = INTERNAL_GENERIC, parameterNames = {"x"}, behavior = PURE)
150150
public abstract static class IsFinite extends Adapter {
151151

152152
static {
@@ -184,7 +184,7 @@ protected RLogicalVector doIsFinite(RAbstractComplexVector vec) {
184184
}
185185
}
186186

187-
@RBuiltin(name = "is.infinite", kind = PRIMITIVE, parameterNames = {"x"}, behavior = PURE)
187+
@RBuiltin(name = "is.infinite", kind = PRIMITIVE, dispatch = INTERNAL_GENERIC, parameterNames = {"x"}, behavior = PURE)
188188
public abstract static class IsInfinite extends Adapter {
189189

190190
static {

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/IsTypeFunctions.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,11 @@ protected byte isRecursive(@SuppressWarnings("unused") RExternalPtr obj) {
136136
return RRuntime.LOGICAL_FALSE;
137137
}
138138

139+
@Specialization
140+
protected byte isRecursive(@SuppressWarnings("unused") RSymbol symbol) {
141+
return RRuntime.LOGICAL_FALSE;
142+
}
143+
139144
@Fallback
140145
protected byte isRecursiveFallback(@SuppressWarnings("unused") Object value) {
141146
return RRuntime.LOGICAL_TRUE;

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/GetCallerFrameNode.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ public MaterializedFrame execute(Frame frame) {
6464
}
6565
CallerFrameClosure closure = (CallerFrameClosure) callerFrameObject;
6666
RCaller parent = RArguments.getCall(frame);
67+
// TODO: notifyCallers may not return the same frame as the caller-frame, it returns
68+
// parent frame, i.e. what RCaller.getParent gives, which is correct?
69+
// Moreover, caller frame is explicitly set by S3 dispatch...
6770
MaterializedFrame slowPathFrame = notifyCallers(closure, parent);
6871
if (slowPathFrame != null) {
6972
return slowPathFrame;

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/CastIntegerNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ protected RAbstractIntVector doDoubleVector(RAbstractDoubleVector x,
176176
if (useClosure()) {
177177
return (RAbstractIntVector) castWithReuse(RType.Integer, operand, naProfile.getConditionProfile());
178178
}
179-
return vectorCopy(operand, naCheck.convertDoubleVectorToIntData(operand), naCheck.neverSeenNA());
179+
return vectorCopy(operand, naCheck.convertDoubleVectorToIntData(operand), naCheck.neverSeenNAOrNaN());
180180
}
181181

182182
@Specialization

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RCaller.java‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
import java.util.function.Supplier;
2626

2727
import com.oracle.truffle.api.frame.Frame;
28+
import com.oracle.truffle.r.runtime.env.REnvironment;
2829
import com.oracle.truffle.r.runtime.nodes.RSyntaxElement;
30+
import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
2931

3032
/**
3133
* Represents the caller of a function and stored in {@link RArguments}. A value of this type never
3234
* appears in a Truffle execution. Caller remembers its parent caller and frame number as described
33-
* in {@code sys.parent} R function documentation: frames are numbered from 0 (global environment),
34-
* parent does not have to have the frame with number one less, e.g. with do.call(fun, args, envir)
35+
* in {@code sys.parent} R function documentation: frames are numbered from 0 (global environment).
36+
* Parent does not have to have the frame with number one less, e.g. with do.call(fun, args, envir)
3537
* when fun asks for parent, it should get 'envir', moreover, when evaluating promises parent frame
3638
* and frame with number one less are typically also not the same frames. See also builtins in
3739
* {@code FrameFunctions} for more details.
@@ -58,26 +60,42 @@ public final class RCaller {
5860
private final int depth;
5961
private boolean visibility;
6062
private final RCaller parent;
63+
6164
/**
62-
* The payload can be an RSyntaxNode, a {@link Supplier}, or an {@link RCaller} (which marks
63-
* promise evaluation frames, see {@link #isPromise()}). Payload represents the syntax (AST) of
64-
* how the function was invoked. If the function was invoked via regular call node, then the
65-
* syntax can be that call node (RSyntaxNode case), if the function was invoked by other means
66-
* and we do not have the actual syntax for the invocation, we only provide it lazily via
67-
* Supplier, so that we do not have to always construct the AST nodes.
65+
* The payload can be
66+
* <ul>
67+
* <li>{@code null} for top level {@link RCaller} (~global environment)</li>
68+
* <li>{@link RSyntaxNode}</li>
69+
* <li>{@link Supplier} of the {@link RSyntaxNode}</li>
70+
* <li>{@link RCaller} (which marks promise evaluation frames, see {@link #isPromise()})</li>
71+
* <li>{@link REnvironment} (which marks promise evaluation frame with explicit "sys parent",
72+
* see {@link #hasSysParent()})</li>
73+
* </ul>
74+
*
75+
* If the function was invoked via regular call node, then the syntax can be that call node
76+
* (RSyntaxNode case), if the function was invoked by other means and we do not have the actual
77+
* syntax for the invocation, we only provide it lazily via Supplier, so that we do not have to
78+
* always construct the AST nodes. {@link RCaller} with other types of {@link #payload} are used
79+
* for promise frames or other artificial situations.
80+
*
81+
* Note on promise evaluation frame with explicit "sys parent": the "sys parent" frame does not
82+
* have to be explicit if the environment has valid {@link RCaller} in its arguments array,
83+
* which is the case if the environment represents a frame of a function that is on the call
84+
* stack. If the environment does not come from currently evaluated function (e.g. manually
85+
* constructed), then we cannot use {@link RCaller} to identify it, since there's no
86+
* {@link #depth} that would point to the corresponding frame.
6887
*/
6988
private final Object payload;
7089

71-
private RCaller(Frame callingFrame, Object nodeOrSupplier) {
72-
this.depth = depthFromFrame(callingFrame);
73-
this.parent = parentFromFrame(callingFrame);
74-
this.payload = nodeOrSupplier;
90+
private RCaller(Frame callingFrame, Object payload) {
91+
this(depthFromFrame(callingFrame), parentFromFrame(callingFrame), payload);
7592
}
7693

77-
private RCaller(int depth, RCaller parent, Object nodeOrSupplier) {
94+
private RCaller(int depth, RCaller parent, Object payload) {
95+
assert payload == null || payload instanceof Supplier<?> || payload instanceof RCaller || payload instanceof REnvironment || payload instanceof RSyntaxNode : payload;
7896
this.depth = depth;
7997
this.parent = parent;
80-
this.payload = nodeOrSupplier;
98+
this.payload = payload;
8199
}
82100

83101
private static int depthFromFrame(Frame callingFrame) {
@@ -129,11 +147,21 @@ public boolean isValidCaller() {
129147
* parent from there.
130148
*/
131149
public boolean isPromise() {
132-
return payload instanceof RCaller;
150+
return (payload instanceof RCaller) || (payload instanceof REnvironment);
133151
}
134152

135-
public RCaller getPromiseOriginalCall() {
136-
return (RCaller) payload;
153+
/**
154+
* If the {@link RCaller} instance {@link #isPromise()}, then it may have explicitly set
155+
* "sys parent", which is what {@code parent.frame} should return. See {@code ParentFrame} built
156+
* in for details.
157+
*/
158+
public boolean hasSysParent() {
159+
return payload instanceof REnvironment;
160+
}
161+
162+
public REnvironment getSysParent() {
163+
assert payload instanceof REnvironment;
164+
return (REnvironment) payload;
137165
}
138166

139167
public static RCaller createInvalid(Frame callingFrame) {
@@ -175,6 +203,11 @@ public static RCaller createForPromise(RCaller originalCaller, Frame frame) {
175203
return new RCaller(newDepth, originalCaller, originalCall);
176204
}
177205

206+
public static RCaller createForPromise(RCaller originalCaller, Frame frame, REnvironment sysParent) {
207+
int newDepth = frame == null ? 0 : RArguments.getDepth(frame);
208+
return new RCaller(newDepth, originalCaller, sysParent);
209+
}
210+
178211
public boolean getVisibility() {
179212
return visibility;
180213
}

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/Utils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ public T visitFrame(FrameInstance frameInstance) {
437437
* Retrieve the caller frame of the current frame.
438438
*
439439
* TODO Calls to this method should be validated with respect to whether promise evaluation is
440-
* in progress and replaced with use of {@code FrameDepthNode}.
440+
* in progress.
441441
*/
442442
public static Frame getCallerFrame(RCaller caller, FrameAccess fa) {
443443
RCaller parent = caller;

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ops/na/NACheck.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,10 @@ public boolean neverSeenNA() {
228228
return state != CHECK && !seenNaN;
229229
}
230230

231+
public boolean neverSeenNAOrNaN() {
232+
return neverSeenNA() && seenNaN;
233+
}
234+
231235
public boolean hasNeverBeenTrue() {
232236
return neverSeenNA();
233237
}

0 commit comments

Comments
 (0)