[GR-44830] Only catch RaiseException for ReferenceProcessingService and improve defined?()

PullRequest: truffleruby/3876

Author: eregon

spec/ruby/language/defined_spec.rb

@@ -180,6 +180,32 @@
       ScratchPad.recorded.should == :defined_specs_fixnum_method
     end
   end
+
+  describe "having a throw in the receiver" do
+    it "escapes defined? and performs the throw semantics as normal" do
+      defined_returned = false
+      catch(:out) {
+        # NOTE: defined? behaves differently if it is called in a void context, see below
+        defined?(throw(:out, 42).foo).should == :unreachable
+        defined_returned = true
+      }.should == 42
+      defined_returned.should == false
+    end
+  end
+
+  describe "in a void context" do
+    it "does not execute the receiver" do
+      ScratchPad.record :not_executed
+      defined?(DefinedSpecs.side_effects / 2)
+      ScratchPad.recorded.should == :not_executed
+    end
+
+    it "warns about the void context when parsing it" do
+      -> {
+        eval "defined?(DefinedSpecs.side_effects / 2); 42"
+      }.should complain("(eval):1: warning: possibly useless use of defined? in void context\n", verbose: true)
+    end
+  end
 end
 
 describe "The defined? keyword for an expression" do

spec/tags/language/defined_tags.txt

@@ -8,3 +8,4 @@ fails:The defined? keyword for a variable scoped constant returns nil if the ins
 fails:The defined? keyword for a variable scoped constant returns nil if the global scoped constant is not defined
 fails:The defined? keyword for a variable scoped constant returns nil if the class scoped constant is not defined
 fails:The defined? keyword for a variable scoped constant returns nil if the local scoped constant is not defined
+fails:The defined? keyword when called with a method name in a void context warns about the void context when parsing it

src/main/java/org/truffleruby/core/ReferenceProcessingService.java

@@ -20,7 +20,6 @@
 import org.truffleruby.language.control.RaiseException;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import org.truffleruby.language.control.TerminationException;
 
 public abstract class ReferenceProcessingService<R extends ReferenceProcessingService.PhantomProcessingReference<R, T>, T> {
 
@@ -168,13 +167,6 @@ protected void runCatchingErrors(RubyContext context, RubyLanguage language,
             action.accept(context, language, reference);
         } catch (RaiseException e) {
             context.getCoreExceptions().showExceptionIfDebug(e.getException());
-        } catch (TerminationException e) {
-            throw e;
-        } catch (Exception e) {
-            // Do nothing, the finalizer thread must continue to process objects.
-            if (context.getCoreLibrary().getDebug() == Boolean.TRUE) {
-                e.printStackTrace();
-            }
         }
     }
 

src/main/java/org/truffleruby/language/defined/DefinedNode.java

@@ -27,6 +27,11 @@ public Object execute(VirtualFrame frame) {
         return child.isDefined(frame, getLanguage(), getContext());
     }
 
+    @Override
+    public void doExecuteVoid(VirtualFrame frame) {
+        // do nothing
+    }
+
     @Override
     public RubyNode cloneUninitialized() {
         var copy = new DefinedNode(child.cloneUninitialized());

src/main/java/org/truffleruby/language/dispatch/RubyCallNode.java

@@ -29,6 +29,7 @@
 import org.truffleruby.language.arguments.KeywordArgumentsDescriptorManager;
 import org.truffleruby.language.arguments.RubyArguments;
 import org.truffleruby.language.arguments.SplatToArgsNode;
+import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.literal.NilLiteralNode;
 import org.truffleruby.language.methods.BlockDefinitionNode;
 import org.truffleruby.language.methods.InternalMethod;
@@ -321,10 +322,9 @@ public Object isDefined(VirtualFrame frame, RubyContext context) {
             allArgumentsDefinedProfile.enter();
 
             final Object receiverObject;
-
             try {
                 receiverObject = receiver.execute(frame);
-            } catch (Exception e) {
+            } catch (RaiseException e) {
                 receiverExceptionProfile.enter();
                 return nil;
             }