Skip to content

Commit

Permalink
[Truffle] Simplified the exception_errno_error primitive.
Browse files Browse the repository at this point in the history
@nirvdrum
nirvdrum committed Sep 14, 2015
1 parent c38e5ce commit 089d126
Showing 2 changed files with 2 additions and 153 deletions.
119 changes: 2 additions & 117 deletions truffle/src/main/java/org/jruby/truffle/nodes/rubinius/ExceptionPrimitiveNodes.java
View file
Original file line number Diff line number Diff line change
@@ -25,130 +25,15 @@ public abstract class ExceptionPrimitiveNodes {
@RubiniusPrimitive(name = "exception_errno_error", needsSelf = false)
public static abstract class ExceptionErrnoErrorPrimitiveNode extends RubiniusPrimitiveNode {

// If you add a constant here, add it below in isExceptionSupported() too.
protected final static int EPERM = Errno.EPERM.intValue();
protected final static int ENOENT = Errno.ENOENT.intValue();
protected final static int EBADF = Errno.EBADF.intValue();
protected final static int EEXIST = Errno.EEXIST.intValue();
protected final static int EACCES = Errno.EACCES.intValue();
protected final static int EFAULT = Errno.EFAULT.intValue();
protected final static int ENOTDIR = Errno.ENOTDIR.intValue();
protected final static int EINVAL = Errno.EINVAL.intValue();
protected final static int EINPROGRESS = Errno.EINPROGRESS.intValue();
protected final static int ENOTCONN = Errno.ENOTCONN.intValue();
protected final static int ECONNREFUSED = Errno.ECONNREFUSED.intValue();

public static boolean isExceptionSupported(int errno) {
return errno == EPERM || errno == ENOENT || errno == EBADF || errno == EEXIST || errno == EACCES
|| errno == EFAULT || errno == ENOTDIR || errno == EINVAL || errno == EINPROGRESS
|| errno == ENOTCONN || errno == ECONNREFUSED;
}

public ExceptionErrnoErrorPrimitiveNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
}

@Specialization(guards = {"isRubyString(message)", "errno == EPERM"})
public DynamicObject eperm(DynamicObject message, int errno) {
return getContext().getCoreLibrary().operationNotPermittedError(message.toString(), this);
}

@Specialization(guards = {"errno == EPERM", "isNil(message)"})
public DynamicObject eperm(Object message, int errno) {
return getContext().getCoreLibrary().operationNotPermittedError("nil", this);
}

@Specialization(guards = {"isRubyString(message)", "errno == ENOENT"})
public DynamicObject enoent(DynamicObject message, int errno) {
return getContext().getCoreLibrary().fileNotFoundError(message.toString(), this);
}

@Specialization(guards = { "errno == ENOENT", "isNil(message)" })
public DynamicObject enoent(Object message, int errno) {
return getContext().getCoreLibrary().fileNotFoundError("nil", this);
}

@Specialization(guards = { "errno == EBADF", "isNil(message)" })
public DynamicObject ebadf(Object message, int errno) {
return getContext().getCoreLibrary().badFileDescriptor(this);
}

@Specialization(guards = {"isRubyString(message)", "errno == EEXIST"})
public DynamicObject eexist(DynamicObject message, int errno) {
return getContext().getCoreLibrary().fileExistsError(message.toString(), this);
}

@Specialization(guards = { "errno == EEXIST", "isNil(message)" })
public DynamicObject eexist(Object message, int errno) {
return getContext().getCoreLibrary().fileExistsError("nil", this);
}

@Specialization(guards = {"isRubyString(message)", "errno == EACCES"})
public DynamicObject eacces(DynamicObject message, int errno) {
return getContext().getCoreLibrary().permissionDeniedError(message.toString(), this);
}

@Specialization(guards = {"errno == EACCES", "isNil(message)"})
public DynamicObject eacces(Object message, int errno) {
return getContext().getCoreLibrary().permissionDeniedError("nil", this);
}

@Specialization(guards = {"isRubyString(message)", "errno == EFAULT"})
public DynamicObject efault(DynamicObject message, int errno) {
return getContext().getCoreLibrary().badAddressError(this);
}

@Specialization(guards = {"errno == EFAULT", "isNil(message)"})
public DynamicObject efault(Object message, int errno) {
return getContext().getCoreLibrary().badAddressError(this);
}

@Specialization(guards = {"isRubyString(message)", "errno == ENOTDIR"})
public DynamicObject enotdir(DynamicObject message, int errno) {
return getContext().getCoreLibrary().notDirectoryError(message.toString(), this);
}

@Specialization(guards = {"isRubyString(message)", "errno == EINVAL"})
public DynamicObject einval(DynamicObject message, int errno) {
return getContext().getCoreLibrary().errnoError(errno, this);
}

@Specialization(guards = {"isRubyString(message)", "errno == EINPROGRESS"})
public DynamicObject einprogress(DynamicObject message, int errno) {
@Specialization
public DynamicObject exceptionErrnoError(DynamicObject message, int errno) {
return getContext().getCoreLibrary().errnoError(errno, this);
}

@Specialization(guards = {"isRubyString(message)", "errno == ENOTCONN"})
public DynamicObject enotconn(DynamicObject message, int errno) {
return getContext().getCoreLibrary().errnoError(errno, this);
}

@Specialization(guards = {"isRubyString(message)", "errno == ECONNREFUSED"})
public DynamicObject econnrefused(DynamicObject message, int errno) {
return getContext().getCoreLibrary().errnoError(errno, this);
}

@TruffleBoundary
@Specialization(guards = "!isExceptionSupported(errno)")
public DynamicObject unsupported(Object message, int errno) {
final Errno errnoObject = Errno.valueOf(errno);

final String messageString;
if (RubyGuards.isRubyString(message)) {
messageString = message.toString();
} else if (message == nil()) {
messageString = "nil";
} else {
messageString = "unsupported message type";
}

if (errnoObject == null) {
throw new UnsupportedOperationException("errno: " + errno + " " + messageString);
} else {
throw new UnsupportedOperationException("errno: " + errnoObject.name());
}
}

}

}
36 changes: 0 additions & 36 deletions truffle/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java
View file
Original file line number Diff line number Diff line change
@@ -1192,47 +1192,11 @@ public DynamicObject ioError(String fileName, Node currentNode) {
return ExceptionNodes.createRubyException(ioErrorClass, Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("Error reading file - %s", fileName), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject badAddressError(Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.EFAULT), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist("Bad address", UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}


public DynamicObject badFileDescriptor(Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.EBADF), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist("Bad file descriptor", UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject fileExistsError(String fileName, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.EEXIST), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("File exists - %s", fileName), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject fileNotFoundError(String fileName, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.ENOENT), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("No such file or directory - %s", fileName), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject dirNotEmptyError(String path, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.ENOTEMPTY), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("Directory not empty - %s", path), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject operationNotPermittedError(String path, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.EPERM), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("Operation not permitted - %s", path), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject permissionDeniedError(String path, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.EACCES), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("Permission denied - %s", path), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject notDirectoryError(String path, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
return ExceptionNodes.createRubyException(getErrnoClass(Errno.ENOTDIR), Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(stringClass), RubyString.encodeBytelist(String.format("Not a directory - %s", path), UTF8Encoding.INSTANCE), StringSupport.CR_UNKNOWN, null), RubyCallStack.getBacktrace(currentNode));
}

public DynamicObject rangeError(int code, DynamicObject encoding, Node currentNode) {
CompilerAsserts.neverPartOfCompilation();
assert RubyGuards.isRubyEncoding(encoding);

5 comments on commit 089d126

@eregon
Copy link
Member

@eregon eregon commented on 089d126 Sep 15, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This discards the message given and the messages hard-coded in CoreLibrary, is that not a problem?

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a problem. We were already ignoring it in half the specializations (note: I probably wrote those, so that may not be a fair assessment). For whatever reason, Rubinius seems to build up its own error messages that sometimes aren't as detailed as the system-provided ones. While there is potential to maybe miss some context or a special-formed message that's easy to grep for, I think the trade-off of a single specialization that uses the same error messages consistently has value. This new approach always routes through jnr-constants, which uses strerror(3) to get the error message.

@eregon
Copy link
Member

@eregon eregon commented on 089d126 Sep 15, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the filename for instance should be shown on ENOENT, which is not the case anymore:

$ jt ruby -e 'File.open "notexist"'
.../ruby/core/rubinius/common/exception.rb:389:in `errno_error': No such file or directory (Errno::ENOENT)
BEFORE:
$ jt ruby -e 'File.open "notexist"'
.../ruby/core/rubinius/common/exception.rb:389:in `errno_error': No such file or directory -  notexist (Errno::ENOENT)

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I can roll some of these back. I'd still like to go with the general approach. The feedback loop of adding one sys error at a time is getting a bit cumbersome otherwise.

Sorry, something went wrong.

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 893d10b strikes up the balance we're looking for. If you find another case that's not quite right, please let me know. That commit more closely matches what Rubinius does.

Sorry, something went wrong.

Please sign in to comment.