Skip to content

Commit 728e3d8

Browse files
author
Nicolas Laurent
committed
defined? must swallow all exceptions
This further lets us simplify the logic, as it's no longer requirement to wrap exceptions from autoloaded modules.
1 parent 5063d7b commit 728e3d8

File tree

3 files changed

+14
-63
lines changed

3 files changed

+14
-63
lines changed

src/main/java/org/truffleruby/language/constants/OrAssignConstantNode.java

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,12 @@ public Object execute(VirtualFrame frame) {
4444
return writeConstant.execute(frame);
4545
}
4646

47-
// Conceptually, we want to rewrite `<x>::Foo ||= <y>` to `(<x>.isDefined && <x>) || (<x>::Foo = <y>)`
47+
// Conceptually, we want to rewrite `<x>::Foo ||= <y>` to `(defined?(<x>) && <x>) || (<x>::Foo = <y>)`
4848
// BUT, we want the side-effects of <x> (the module part) to be only triggered once.
49-
//
50-
// To do so, we start by evaluating the module part.
51-
// Possible issues:
52-
// - An exception is raised (not during module autoloading)
53-
// -> let it go through
54-
// - An exception is raised (during module autoloading)
55-
// -> capture the exception, evaluate the right-hand, then throw the exception.
56-
// Normally, this would be swallowed by `isDefined`, but because it causes the left of `||` to be false,
57-
// it will be thrown again when evaluating the left-hand side of the assignment.
58-
// We do need to evaluate the right-hand side because of Ruby semantics.
59-
final RubyModule module;
60-
try {
61-
module = readConstant.evaluateModuleWrappingAutoloadExceptions(frame);
62-
} catch (AutoloadException e) {
63-
writeConstant.evaluateValue(frame);
64-
throw e.raiseException;
65-
}
49+
// We do let any exception raised bubble through. Normally they would be swallowed by `defined?`, but
50+
// MRI raises them anyway, *before* evaluation the right-hand side (which is a different behaviour from
51+
// regular constant assignment, which evaluates the right-hand side first).
52+
final RubyModule module = readConstant.evaluateModule(frame);
6653

6754
// Next we check if the constant itself is defined, and if it is, we get its value.
6855
final RubyConstant constant = readConstant.getConstantIfDefined(module);

src/main/java/org/truffleruby/language/constants/ReadConstantNode.java

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -75,31 +75,9 @@ private LookupConstantNode getLookupConstantNode() {
7575
return lookupConstantNode;
7676
}
7777

78-
/** Reads the constant value, wrapping any exception occuring during module autoloading in an
79-
* {@link AutoloadException}. */
80-
private Object readConstantWrappingAutoloadExceptions(VirtualFrame frame) {
81-
final Object moduleObject = moduleNode instanceof ReadConstantNode
82-
? ((ReadConstantNode) moduleNode).readConstantWrappingAutoloadExceptions(frame)
83-
: moduleNode.execute(frame);
84-
final RubyModule module = checkModule(moduleObject);
85-
final RubyConstant constant = getLookupConstantNode().lookupConstant(LexicalScope.IGNORE, module, name, true);
86-
try {
87-
return getConstant(module, constant);
88-
} catch (RaiseException e) {
89-
if (constant.isAutoload()) {
90-
throw new AutoloadException(e);
91-
}
92-
throw e;
93-
}
94-
}
95-
96-
/** Returns the evaluated module part, wrapping any exception occuring during module autoloading in an
97-
* {@link AutoloadException}. */
98-
public RubyModule evaluateModuleWrappingAutoloadExceptions(VirtualFrame frame) {
99-
final Object moduleObject = moduleNode instanceof ReadConstantNode
100-
? ((ReadConstantNode) moduleNode).readConstantWrappingAutoloadExceptions(frame)
101-
: moduleNode.execute(frame);
102-
return checkModule(moduleObject);
78+
/** Evaluate the module part of the constant read. */
79+
public RubyModule evaluateModule(VirtualFrame frame) {
80+
return checkModule(moduleNode.execute(frame));
10381
}
10482

10583
/** Whether the module part of this constant read is undefined, without attempting to evaluate it. */
@@ -109,26 +87,16 @@ public boolean isModuleTriviallyUndefined(VirtualFrame frame, RubyLanguage langu
10987

11088
@Override
11189
public Object isDefined(VirtualFrame frame, RubyLanguage language, RubyContext context) {
112-
final RubyConstant constant = getConstantIfDefined(frame, language, context);
113-
return constant == null ? nil : coreStrings().CONSTANT.createInstance(getContext());
114-
}
115-
116-
/** Returns the constant, it it is defined. Otherwise returns {@code null}. */
117-
private RubyConstant getConstantIfDefined(VirtualFrame frame, RubyLanguage language, RubyContext context) {
11890
if (isModuleTriviallyUndefined(frame, language, context)) {
119-
return null;
91+
return nil;
12092
}
121-
122-
final RubyModule module;
12393
try {
124-
module = evaluateModuleWrappingAutoloadExceptions(frame);
125-
} catch (AutoloadException e) {
126-
// If autoloading a module raised an exception,
127-
// MRI dictates that we should swallow the exception and return `nil`.
128-
return null;
94+
final RubyModule module = checkModule(moduleNode.execute(frame));
95+
final RubyConstant constant = getConstantIfDefined(module);
96+
return constant == null ? nil : coreStrings().CONSTANT.createInstance(getContext());
97+
} catch (RaiseException e) {
98+
return nil; // MRI swallows all exceptions in defined? (MRI Bug#5786)
12999
}
130-
131-
return getConstantIfDefined(module);
132100
}
133101

134102
/** Given the module, returns the constant, it it is defined. Otherwise returns {@code null}. */

src/main/java/org/truffleruby/language/constants/WriteConstantNode.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@ public Object execute(VirtualFrame frame, RubyModule module) {
5151
return value;
5252
}
5353

54-
public void evaluateValue(VirtualFrame frame) {
55-
valueNode.execute(frame);
56-
}
57-
5854
@Override
5955
public void assign(VirtualFrame frame, Object value) {
6056
final Object moduleObject = moduleNode.execute(frame);

0 commit comments

Comments
 (0)