Skip to content

Conversation

@cyangle
Copy link
Contributor

@cyangle cyangle commented Dec 9, 2025

Try to fix #16486

Reverting back to the original fix.

Summary

I've successfully fixed the Crystal interpreter bug where type narrowing failed when:

Inside an if block with a return statement
A union type is narrowed with a nil check and return
The narrowed variable is used in a method call

Root Cause

The interpreter's visit(node : Expressions) method didn't set the node.type. The semantic pass sets this via the bind_to method, but in the interpreter context this binding doesn't automatically happen. When an if statement's then/else branches (Expressions nodes) were used later, the interpreter would fail trying to access their type.

The Fix

Added code to the visit(node : Expressions) method to ensure the Expressions node has a type set. The fix:

  • Only sets the type if it's not already set (to avoid overwriting)
  • Sets it to the type of the last expression in the Expressions
  • Sets it to nil_type if the Expressions is empty

This matches what the semantic pass does via its binding system.

Similar to compiler's implementation in Crystal::CodeGenVisitor#codegen_if_branch
@cyangle cyangle marked this pull request as ready for review December 9, 2025 04:16
@cyangle cyangle marked this pull request as draft December 9, 2025 04:35
@cyangle
Copy link
Contributor Author

cyangle commented Dec 9, 2025

I have no idea. Maybe the first approach is the correct fix:

diff --git a/src/compiler/crystal/interpreter/compiler.cr b/src/compiler/crystal/interpreter/compiler.cr
index bba9d5093..da3b875a8 100644
--- a/src/compiler/crystal/interpreter/compiler.cr
+++ b/src/compiler/crystal/interpreter/compiler.cr
@@ -591,6 +591,17 @@ class Crystal::Repl::Compiler < Crystal::Visitor
 
     @wants_value = old_wants_value
 
+    # Ensure the Expressions node has a type. The semantic pass sets this via
+    # bind_to, but in some interpreter contexts it might not be set yet.
+    # Only set if not already set to avoid overwriting.
+    unless node.type?
+      if node.expressions.empty?
+        node.type = @context.program.nil_type
+      else
+        node.type = node.expressions.last.type?
+      end
+    end
+
     false
   end
 

There's a TODO comment acknowledging that there's something wrong with semantic pass:

# TODO: for some reason the semantic pass might leave this as nil
if @wants_value && (then_type = node.then.type?)
upcast node.then, then_type, node.type
end

@cyangle
Copy link
Contributor Author

cyangle commented Dec 9, 2025

Alternative fix is not robust. It still fails for the non-reduced code.

The first fix works for both reduced and non-reduced code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interpreter crashes with error: has no type (Exception)

1 participant