Skip to content

Commit fa79549

Browse files
steve-ssteve-s
committed
[GR-13168] Fix oldClass assign builtin.
PullRequest: fastr/1858
2 parents dd18d93 + dc74aae commit fa79549

File tree

7 files changed

+115
-100
lines changed

7 files changed

+115
-100
lines changed

‎com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/REngine.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,10 @@ private List<RSyntaxNode> parseSource(Source source) throws ParseException {
281281
@Override
282282
public RExpression parse(Source source) throws ParseException {
283283
List<RSyntaxNode> list = parseSource(source);
284-
Object[] data = list.stream().map(node -> RASTUtils.createLanguageElement(node)).toArray();
284+
Object[] data = new Object[list.size()];
285+
for (int i = 0; i < data.length; i++) {
286+
data[i] = RASTUtils.createLanguageElement(list.get(i));
287+
}
285288
return RDataFactory.createExpression(data);
286289
}
287290

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

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,70 +23,51 @@
2323

2424
package com.oracle.truffle.r.nodes.builtin.base;
2525

26+
import static com.oracle.truffle.r.nodes.builtin.CastBuilder.Predef.stringValue;
2627
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
2728
import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.PRIMITIVE;
2829

2930
import com.oracle.truffle.api.CompilerDirectives;
3031
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
32+
import com.oracle.truffle.api.dsl.Fallback;
3133
import com.oracle.truffle.api.dsl.Specialization;
3234
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetClassAttributeNode;
3335
import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
34-
import com.oracle.truffle.r.nodes.unary.CastStringNode;
35-
import com.oracle.truffle.r.nodes.unary.CastStringNodeGen;
36+
import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
3637
import com.oracle.truffle.r.runtime.RError.Message;
38+
import com.oracle.truffle.r.runtime.Utils;
3739
import com.oracle.truffle.r.runtime.builtins.RBuiltin;
38-
import com.oracle.truffle.r.runtime.data.RDataFactory;
40+
import com.oracle.truffle.r.runtime.data.RAttributable;
3941
import com.oracle.truffle.r.runtime.data.RNull;
40-
import com.oracle.truffle.r.runtime.data.RStringVector;
41-
import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
42-
import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
42+
import com.oracle.truffle.r.runtime.data.model.RAbstractStringVector;
4343

4444
// oldClass<- (as opposed to class<-), simply sets the attribute (without handling "implicit" attributes)
4545
@RBuiltin(name = "oldClass<-", kind = PRIMITIVE, parameterNames = {"x", "value"}, behavior = PURE)
4646
public abstract class UpdateOldClass extends RBuiltinNode.Arg2 {
4747

48-
@Child private CastStringNode castStringNode;
4948
@Child private SetClassAttributeNode setClassAttributeNode = SetClassAttributeNode.create();
49+
@Child private ShareObjectNode shareObjectNode;
5050

5151
static {
52-
Casts.noCasts(UpdateOldClass.class);
52+
Casts casts = new Casts(UpdateOldClass.class);
53+
casts.arg("x").asAttributable(true, true, true);
54+
casts.arg("value").allowNull().mustBe(stringValue(), Message.SET_INVALID_CLASS_ATTR).asStringVector();
5355
}
5456

55-
@Specialization(guards = "!isRAbstractStringVector(className)")
56-
protected Object setOldClass(RAbstractContainer arg, RAbstractVector className) {
57+
@Specialization
58+
protected Object setOldClass(RAttributable arg, RAbstractStringVector className) {
5759
if (className.getLength() == 0) {
5860
return setOldClass(arg, RNull.instance);
5961
}
60-
initCastStringNode();
61-
Object result = castStringNode.doCast(className);
62-
return setOldClass(arg, (RStringVector) result);
63-
}
64-
65-
private void initCastStringNode() {
66-
if (castStringNode == null) {
67-
CompilerDirectives.transferToInterpreterAndInvalidate();
68-
castStringNode = insert(CastStringNodeGen.create(false, false, false));
69-
}
70-
}
71-
72-
@Specialization
73-
@TruffleBoundary
74-
protected Object setOldClass(RAbstractContainer arg, String className) {
75-
return setOldClass(arg, RDataFactory.createStringVector(className));
76-
}
77-
78-
@Specialization
79-
@TruffleBoundary
80-
protected Object setOldClass(RAbstractContainer arg, RStringVector className) {
81-
RAbstractContainer result = (RAbstractContainer) arg.getNonShared();
62+
RAttributable result = (RAttributable) getShareObjectNode().execute(arg);
8263
setClassAttributeNode.execute(result, className);
8364
return result;
8465
}
8566

8667
@Specialization
8768
@TruffleBoundary
88-
protected Object setOldClass(RAbstractContainer arg, @SuppressWarnings("unused") RNull className) {
89-
RAbstractContainer result = (RAbstractContainer) arg.getNonShared();
69+
protected Object setOldClass(RAttributable arg, @SuppressWarnings("unused") RNull className) {
70+
RAttributable result = (RAttributable) getShareObjectNode().execute(arg);
9071
setClassAttributeNode.reset(result);
9172
return result;
9273
}
@@ -96,8 +77,22 @@ protected Object setOldClass(@SuppressWarnings("unused") RNull arg, @SuppressWar
9677
return RNull.instance;
9778
}
9879

99-
@Specialization
80+
@Specialization(guards = "!isRNull(className)")
10081
protected Object setOldClass(@SuppressWarnings("unused") RNull arg, @SuppressWarnings("unused") Object className) {
10182
throw error(Message.INVALID_NULL_LHS);
10283
}
84+
85+
@Fallback
86+
@TruffleBoundary
87+
protected Object setOldClass(Object x, @SuppressWarnings("unused") Object className) {
88+
throw error(Message.CANNOT_SET_ATTR_ON, Utils.getTypeName(x));
89+
}
90+
91+
public ShareObjectNode getShareObjectNode() {
92+
if (shareObjectNode == null) {
93+
CompilerDirectives.transferToInterpreterAndInvalidate();
94+
shareObjectNode = insert(ShareObjectNode.create());
95+
}
96+
return shareObjectNode;
97+
}
10398
}

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

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,9 @@
2525
import com.oracle.truffle.api.dsl.Specialization;
2626
import com.oracle.truffle.r.runtime.RType;
2727
import com.oracle.truffle.r.runtime.data.RAttributable;
28-
import com.oracle.truffle.r.runtime.data.RFunction;
2928
import com.oracle.truffle.r.runtime.data.RMissing;
3029
import com.oracle.truffle.r.runtime.data.RNull;
31-
import com.oracle.truffle.r.runtime.data.RS4Object;
32-
import com.oracle.truffle.r.runtime.data.RSymbol;
3330
import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
34-
import com.oracle.truffle.r.runtime.env.REnvironment;
3531

3632
public abstract class CastToAttributableNode extends CastBaseNode {
3733

@@ -60,28 +56,14 @@ protected RMissing cast(@SuppressWarnings("unused") RMissing rmissing) {
6056
return RMissing.instance;
6157
}
6258

59+
// This specialization causes boxing of primitive types to a container (via Truffle DSL)
6360
@Specialization
6461
protected RAttributable cast(RAbstractContainer container) {
6562
return container;
6663
}
6764

68-
@Specialization
69-
protected RAttributable cast(REnvironment environment) {
70-
return environment;
71-
}
72-
73-
@Specialization
74-
protected RAttributable cast(RFunction function) {
75-
return function;
76-
}
77-
78-
@Specialization
79-
protected RAttributable cast(RSymbol symbol) {
80-
return symbol;
81-
}
82-
83-
@Specialization
84-
protected RAttributable cast(RS4Object object) {
85-
return object;
65+
@Specialization(guards = "!isRAbstractContainer(x)")
66+
protected RAttributable cast(RAttributable x) {
67+
return x;
8668
}
8769
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,7 @@ public enum Message {
979979
LENGTH_OF_NULL_UNCHANGED("length of NULL cannot be changed"),
980980
CANNOT_SET_LENGTH("cannot set length of non-(vector or list)"),
981981
LONG_VECTOR_NOT_SUPPORTED("long vector '%s' is not supported"),
982+
CANNOT_SET_ATTR_ON("cannot set attribute on a %s"),
982983
CANNOT_ALLOCATE_VECTOR_GB("cannot allocate vector of size %.1f Gb");
983984

984985
public final String message;

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

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -166,59 +166,63 @@ public static void reportError(Throwable t) {
166166

167167
@TruffleBoundary
168168
private static void reportErrorDefault(Throwable t, int contextId) {
169-
String errMsg = t instanceof RInternalError && t.getMessage() != null && !t.getMessage().isEmpty() ? t.getMessage() : t.getClass().getSimpleName();
169+
boolean detailedMessage = FastROptions.PrintErrorStacktraces.getBooleanValue() || FastROptions.PrintErrorStacktracesToFile.getBooleanValue();
170+
String errMsg = ".";
171+
if (detailedMessage) {
172+
errMsg = ": \"";
173+
errMsg += t instanceof RInternalError && t.getMessage() != null && !t.getMessage().isEmpty() ? t.getMessage() : t.getClass().getSimpleName();
174+
errMsg += "\"";
175+
}
170176
reportError(errMsg, t, contextId);
171177
}
172178

173179
private static void reportError(String errMsg, Throwable throwable, int contextId) {
174180
try {
175181
Throwable t = throwable;
176-
if (FastROptions.PrintErrorStacktracesToFile.getBooleanValue() || FastROptions.PrintErrorStacktraces.getBooleanValue()) {
177-
ByteArrayOutputStream out = new ByteArrayOutputStream();
178-
t.printStackTrace(new PrintStream(out));
179-
String verboseStackTrace;
180-
if (t.getCause() != null && t instanceof IOException) {
181-
t = t.getCause();
182-
}
183-
if (t instanceof RInternalError) {
184-
verboseStackTrace = ((RInternalError) t).getVerboseStackTrace();
185-
} else if (t instanceof RError) {
186-
verboseStackTrace = ((RError) t).getVerboseStackTrace();
187-
} else {
188-
verboseStackTrace = "";
189-
}
190-
if (FastROptions.PrintErrorStacktraces.getBooleanValue()) {
191-
System.err.println(out.toString());
192-
System.err.println(verboseStackTrace);
193-
}
182+
ByteArrayOutputStream out = new ByteArrayOutputStream();
183+
t.printStackTrace(new PrintStream(out));
184+
String verboseStackTrace;
185+
if (t.getCause() != null && t instanceof IOException) {
186+
t = t.getCause();
187+
}
188+
if (t instanceof RInternalError) {
189+
verboseStackTrace = ((RInternalError) t).getVerboseStackTrace();
190+
} else if (t instanceof RError) {
191+
verboseStackTrace = ((RError) t).getVerboseStackTrace();
192+
} else {
193+
verboseStackTrace = "";
194+
}
195+
if (FastROptions.PrintErrorStacktraces.getBooleanValue()) {
196+
System.err.println(out.toString());
197+
System.err.println(verboseStackTrace);
198+
}
194199

195-
String message = "An internal error occurred: \"" + errMsg + "\"\nPlease report an issue at https://github.com/oracle/fastr including the commands";
196-
if (FastROptions.PrintErrorStacktracesToFile.getBooleanValue()) {
197-
Path logfile = Utils.getLogPath(getLogFileName(contextId));
198-
if (logfile != null) {
199-
message += " and the error log file '" + logfile + "'.";
200-
try (BufferedWriter writer = Files.newBufferedWriter(logfile, StandardCharsets.UTF_8, StandardOpenOption.APPEND,
201-
StandardOpenOption.CREATE)) {
202-
writer.append(new Date().toString()).append('\n');
203-
writer.append(out.toString()).append('\n');
204-
writer.append(verboseStackTrace).append("\n\n");
205-
} catch (IOException e) {
206-
e.printStackTrace();
207-
}
208-
} else {
209-
message += ". Cannot write error log file (tried current working directory, user home directory, FastR home directory).";
210-
}
211-
System.err.println(message);
212-
if (RContext.isEmbedded()) {
213-
RSuicide.rSuicide("FastR internal error");
200+
String message = "An internal error occurred" + errMsg + "\nPlease report an issue at https://github.com/oracle/fastr including the commands";
201+
if (FastROptions.PrintErrorStacktracesToFile.getBooleanValue()) {
202+
Path logfile = Utils.getLogPath(getLogFileName(contextId));
203+
if (logfile != null) {
204+
message += " and the error log file '" + logfile + "'.";
205+
try (BufferedWriter writer = Files.newBufferedWriter(logfile, StandardCharsets.UTF_8, StandardOpenOption.APPEND,
206+
StandardOpenOption.CREATE)) {
207+
writer.append(new Date().toString()).append('\n');
208+
writer.append(out.toString()).append('\n');
209+
writer.append(verboseStackTrace).append("\n\n");
210+
} catch (IOException e) {
211+
e.printStackTrace();
214212
}
215213
} else {
216-
message += ". You can rerun FastR with --jvm.DR:+" + FastROptions.PrintErrorStacktracesToFile.name() +
217-
" to turn on internal errors logging. Please attach the log file to the issue if possible.";
214+
message += ". Cannot write error log file (tried current working directory, user home directory, FastR home directory).";
218215
}
219-
if (!FastROptions.PrintErrorStacktraces.getBooleanValue() && !FastROptions.PrintErrorStacktracesToFile.getBooleanValue()) {
220-
System.err.println(message);
216+
System.err.println(message);
217+
if (RContext.isEmbedded()) {
218+
RSuicide.rSuicide("FastR internal error");
221219
}
220+
} else {
221+
message += ". You can rerun FastR with --jvm.DR:+" + FastROptions.PrintErrorStacktracesToFile.name() +
222+
" to turn on internal errors logging. Please attach the log file to the issue if possible.";
223+
}
224+
if (!FastROptions.PrintErrorStacktraces.getBooleanValue() && !FastROptions.PrintErrorStacktracesToFile.getBooleanValue()) {
225+
System.err.println(message);
222226
}
223227
} catch (ExitException | ThreadDeath t) {
224228
throw t;

‎com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47525,6 +47525,22 @@ NULL
4752547525
#argv <- list(structure(c(4L, 4L, 4L, 4L, 4L, 4L, 4L, 4L, 4L, 4L, 4L, 4L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 6L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 3L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 5L, 5L, 5L, 5L, 5L, 5L, 5L, 5L, 5L, 5L, 5L, 5L), .Label = c('C', 'E', 'D', 'A', 'F', 'B'), class = 'factor', scores = structure(c(14, 16.5, 1.5, 5, 3, 15), .Dim = 6L, .Dimnames = list(c('A', 'B', 'C', 'D', 'E', 'F')))));oldClass(argv[[1]]);
4752647526
[1] "factor"
4752747527

47528+
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssign#
47529+
#{ x <- 1; oldClass(x) <- c('a', 'b'); oldClass(x) }
47530+
[1] "a" "b"
47531+
47532+
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssign#
47533+
#{ x <- as.pairlist(42); oldClass(x)<-'mycls'; oldClass(x) }
47534+
[1] "mycls"
47535+
47536+
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssign#
47537+
#{ x <- function() 42; oldClass(x)<-'mycls'; oldClass(x) }
47538+
[1] "mycls"
47539+
47540+
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssign#
47541+
#{ x <- new.env(); oldClass(x)<-'mycls'; oldClass(x) }
47542+
[1] "mycls"
47543+
4752847544
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssign#
4752947545
#{ x<-1; oldClass(x)<-"foo"; class(x) }
4753047546
[1] "foo"
@@ -47549,6 +47565,10 @@ NULL
4754947565
#{ x<-1; oldClass(x)<-"integer"; oldClass(x) }
4755047566
[1] "integer"
4755147567

47568+
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testOldClassAssignValidation#
47569+
#{ x <- 1; oldClass(x) <- 42; oldClass(x) }
47570+
Error in oldClass(x) <- 42 : attempt to set invalid 'class' attribute
47571+
4755247572
##com.oracle.truffle.r.test.builtins.TestBuiltin_oldClassassign.testoldClassassign1#
4755347573
#argv <- list(list(), NULL);`oldClass<-`(argv[[1]],argv[[2]]);
4755447574
list()

‎com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_oldClassassign.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,15 @@ public void testOldClassAssign() {
4949
assertEval("{ x<-1; oldClass(x)<-\"integer\"; class(x)<-\"integer\"; class(x) }");
5050

5151
assertEval("{ x<-1; oldClass(x)<-\"integer\"; class(x)<-\"integer\"; oldClass(x) }");
52+
53+
assertEval("{ x <- new.env(); oldClass(x)<-'mycls'; oldClass(x) }");
54+
assertEval("{ x <- function() 42; oldClass(x)<-'mycls'; oldClass(x) }");
55+
assertEval("{ x <- as.pairlist(42); oldClass(x)<-'mycls'; oldClass(x) }");
56+
assertEval("{ x <- 1; oldClass(x) <- c('a', 'b'); oldClass(x) }");
57+
}
58+
59+
@Test
60+
public void testOldClassAssignValidation() {
61+
assertEval("{ x <- 1; oldClass(x) <- 42; oldClass(x) }");
5262
}
5363
}

0 commit comments

Comments
 (0)