From d88c013588e4c92d2510af21b88fd14542c5a902 Mon Sep 17 00:00:00 2001
From: Quinton Miller <nicetas.c@gmail.com>
Date: Tue, 30 Jan 2024 20:47:21 +0800
Subject: [PATCH] Ensure `Crystal::Visitor#visit` returns `Bool` (#14266)

---
 samples/compiler/visitor_example.cr           |  1 +
 src/compiler/crystal/codegen/codegen.cr       | 17 +++++++++++---
 src/compiler/crystal/codegen/primitives.cr    |  2 ++
 src/compiler/crystal/interpreter/compiler.cr  |  5 +++--
 src/compiler/crystal/macros/interpreter.cr    |  8 +++++--
 .../crystal/semantic/cleanup_transformer.cr   |  2 ++
 src/compiler/crystal/semantic/main_visitor.cr | 22 ++++++++++++++++---
 .../semantic/restrictions_augmenter.cr        |  3 ++-
 src/compiler/crystal/semantic/to_s.cr         |  3 +++
 .../crystal/semantic/top_level_visitor.cr     |  1 +
 .../crystal/semantic/type_guess_visitor.cr    |  3 +++
 src/compiler/crystal/syntax/to_s.cr           | 15 +++++++++++++
 src/compiler/crystal/tools/expand.cr          |  4 ++++
 src/compiler/crystal/tools/implementations.cr |  2 ++
 .../crystal/tools/print_types_visitor.cr      |  4 ++++
 15 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/samples/compiler/visitor_example.cr b/samples/compiler/visitor_example.cr
index ec063a832efa..d9be4da91baf 100644
--- a/samples/compiler/visitor_example.cr
+++ b/samples/compiler/visitor_example.cr
@@ -15,6 +15,7 @@ class Counter < Crystal::Visitor
 
   def visit(node : Crystal::NumberLiteral)
     @count += 1
+    false
   end
 
   def visit(node : Crystal::ASTNode)
diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr
index a6015012eb7c..e7128302a66e 100644
--- a/src/compiler/crystal/codegen/codegen.cr
+++ b/src/compiler/crystal/codegen/codegen.cr
@@ -501,18 +501,22 @@ module Crystal
 
     def visit(node : Nop)
       @last = llvm_nil
+      false
     end
 
     def visit(node : NilLiteral)
       @last = llvm_nil
+      false
     end
 
     def visit(node : BoolLiteral)
       @last = int1(node.value ? 1 : 0)
+      false
     end
 
     def visit(node : CharLiteral)
       @last = int32(node.value.ord)
+      false
     end
 
     def visit(node : NumberLiteral)
@@ -542,14 +546,17 @@ module Crystal
       in .f64?
         @last = float64(node.value)
       end
+      false
     end
 
     def visit(node : StringLiteral)
       @last = build_string_constant(node.value, node.value)
+      false
     end
 
     def visit(node : SymbolLiteral)
       @last = int(@symbols[node.value])
+      false
     end
 
     def visit(node : TupleLiteral)
@@ -1092,7 +1099,7 @@ module Crystal
 
       request_value(value)
 
-      return if value.no_returns?
+      return false if value.no_returns?
 
       last = @last
 
@@ -1106,7 +1113,7 @@ module Crystal
               read_class_var_ptr(target)
             when Var
               # Can't assign void
-              return if target.type.void?
+              return false if target.type.void?
 
               # If assigning to a special variable in a method that yields,
               # assign to that variable too.
@@ -1284,6 +1291,7 @@ module Crystal
       else
         node.raise "BUG: missing context var: #{node.name}"
       end
+      false
     end
 
     def visit(node : Global)
@@ -1292,6 +1300,7 @@ module Crystal
 
     def visit(node : ClassVar)
       @last = read_class_var(node)
+      false
     end
 
     def visit(node : InstanceVar)
@@ -1403,7 +1412,7 @@ module Crystal
 
       unless filtered_type
         @last = upcast llvm_nil, resulting_type, @program.nil
-        return
+        return false
       end
 
       non_nilable_type = node.non_nilable_type
@@ -1664,6 +1673,7 @@ module Crystal
 
     def visit(node : Unreachable)
       builder.unreachable
+      false
     end
 
     def check_proc_is_not_closure(value, type)
@@ -2049,6 +2059,7 @@ module Crystal
     def unreachable(file = __FILE__, line = __LINE__)
       debug_codegen_log(file, line) { "Reached the unreachable!" }
       builder.unreachable
+      false
     end
 
     def allocate_aggregate(type)
diff --git a/src/compiler/crystal/codegen/primitives.cr b/src/compiler/crystal/codegen/primitives.cr
index 9ed05c0009f2..80ba7d832a41 100644
--- a/src/compiler/crystal/codegen/primitives.cr
+++ b/src/compiler/crystal/codegen/primitives.cr
@@ -11,6 +11,8 @@ class Crystal::CodeGenVisitor
             else
               raise "BUG: unhandled primitive in codegen visit: #{node.name}"
             end
+
+    false
   end
 
   def codegen_primitive(call, node, target_def, call_args)
diff --git a/src/compiler/crystal/interpreter/compiler.cr b/src/compiler/crystal/interpreter/compiler.cr
index bfd1514b419f..04fc885fce7e 100644
--- a/src/compiler/crystal/interpreter/compiler.cr
+++ b/src/compiler/crystal/interpreter/compiler.cr
@@ -762,7 +762,7 @@ class Crystal::Repl::Compiler < Crystal::Visitor
     # particularly when outside of a method.
     if is_self && !scope.is_a?(Program) && !scope.passed_as_self?
       put_type scope, node: node
-      return
+      return false
     end
 
     local_var = lookup_local_var_or_closured_var(node.name)
@@ -1687,7 +1687,7 @@ class Crystal::Repl::Compiler < Crystal::Visitor
     if node.upcast?
       upcast node.obj, obj_type, node.non_nilable_type
       upcast node.obj, node.non_nilable_type, node.type
-      return
+      return false
     end
 
     # Check if obj is a `to_type`
@@ -3158,6 +3158,7 @@ class Crystal::Repl::Compiler < Crystal::Visitor
     {% end %}
 
     call compiled_def, node: node
+    false
   end
 
   def visit(node : ASTNode)
diff --git a/src/compiler/crystal/macros/interpreter.cr b/src/compiler/crystal/macros/interpreter.cr
index 5bc7cd0117ca..1e18e92b7545 100644
--- a/src/compiler/crystal/macros/interpreter.cr
+++ b/src/compiler/crystal/macros/interpreter.cr
@@ -118,6 +118,7 @@ module Crystal
 
     def visit(node : MacroLiteral)
       @str << node.value
+      false
     end
 
     def visit(node : MacroVerbatim)
@@ -135,7 +136,8 @@ module Crystal
     def visit(node : Var)
       var = @vars[node.name]?
       if var
-        return @last = var
+        @last = var
+        return false
       end
 
       # Try to consider the var as a top-level macro call.
@@ -150,7 +152,8 @@ module Crystal
       # and in this case the parser has no idea about this, so the only
       # solution is to do it now.
       if value = interpret_top_level_call?(Call.new(nil, node.name))
-        return @last = value
+        @last = value
+        return false
       end
 
       node.raise "undefined macro variable '#{node.name}'"
@@ -549,6 +552,7 @@ module Crystal
       else
         node.raise "unknown macro instance var: '#{node.name}'"
       end
+      false
     end
 
     def visit(node : TupleLiteral)
diff --git a/src/compiler/crystal/semantic/cleanup_transformer.cr b/src/compiler/crystal/semantic/cleanup_transformer.cr
index bc3084429ed3..541e0f51d662 100644
--- a/src/compiler/crystal/semantic/cleanup_transformer.cr
+++ b/src/compiler/crystal/semantic/cleanup_transformer.cr
@@ -635,10 +635,12 @@ module Crystal
         if @a_def.vars.try &.[node.name]?.try &.closured?
           @vars << node
         end
+        false
       end
 
       def visit(node : InstanceVar)
         @vars << node
+        false
       end
 
       def visit(node : ASTNode)
diff --git a/src/compiler/crystal/semantic/main_visitor.cr b/src/compiler/crystal/semantic/main_visitor.cr
index 37218e7a34dc..61d1aa508a46 100644
--- a/src/compiler/crystal/semantic/main_visitor.cr
+++ b/src/compiler/crystal/semantic/main_visitor.cr
@@ -226,6 +226,8 @@ module Crystal
         node.syntax_replacement = type
         node.bind_to type
       end
+
+      false
     end
 
     def visit(node : Generic)
@@ -353,6 +355,7 @@ module Crystal
 
     def visit(node : Self)
       node.type = the_self(node).instance_type
+      false
     end
 
     def visit(node : Var)
@@ -394,6 +397,7 @@ module Crystal
       else
         node.raise "read before assignment to local variable '#{node.name}'"
       end
+      false
     end
 
     def visit(node : TypeDeclaration)
@@ -628,6 +632,8 @@ module Crystal
         ivar.nil_reason ||= NilReason.new(node.name, :used_before_initialized, [node] of ASTNode)
         ivar.bind_to program.nil_var
       end
+
+      false
     end
 
     def visit(node : ReadInstanceVar)
@@ -1007,7 +1013,7 @@ module Crystal
     end
 
     def visit(node : Block)
-      return if node.visited?
+      return false if node.visited?
 
       node.visited = true
       node.context = current_non_block_context
@@ -1630,6 +1636,7 @@ module Crystal
             ivars[node.name] = node_in_callstack(node)
           end
         end
+        false
       end
 
       def visit(node : Var)
@@ -1761,7 +1768,7 @@ module Crystal
         comp.accept self
         node.syntax_replacement = comp
         node.bind_to comp
-        return
+        return false
       end
 
       if needs_type_filters? && (var = get_expression_var(node.obj))
@@ -2060,7 +2067,7 @@ module Crystal
       unless node.has_breaks?
         if endless_while
           node.type = program.no_return
-          return
+          return false
         end
 
         filter_vars TypeFilters.not(cond_type_filters)
@@ -2325,6 +2332,8 @@ module Crystal
       else
         node.raise "BUG: unhandled primitive in MainVisitor: #{node.name}"
       end
+
+      false
     end
 
     def visit_va_arg(node)
@@ -3042,31 +3051,38 @@ module Crystal
 
     def visit(node : Nop)
       node.type = @program.nil
+      false
     end
 
     def visit(node : NilLiteral)
       node.type = @program.nil
+      false
     end
 
     def visit(node : BoolLiteral)
       node.type = program.bool
+      false
     end
 
     def visit(node : NumberLiteral)
       node.type = program.type_from_literal_kind node.kind
+      false
     end
 
     def visit(node : CharLiteral)
       node.type = program.char
+      false
     end
 
     def visit(node : SymbolLiteral)
       node.type = program.symbol
       program.symbols.add node.value
+      false
     end
 
     def visit(node : StringLiteral)
       node.type = program.string
+      false
     end
 
     def visit(node : RegexLiteral)
diff --git a/src/compiler/crystal/semantic/restrictions_augmenter.cr b/src/compiler/crystal/semantic/restrictions_augmenter.cr
index a627f27f5fe0..5b3e31056110 100644
--- a/src/compiler/crystal/semantic/restrictions_augmenter.cr
+++ b/src/compiler/crystal/semantic/restrictions_augmenter.cr
@@ -59,7 +59,8 @@ module Crystal
 
     def visit(node : Call)
       if expanded = node.expanded
-        return expanded.accept self
+        expanded.accept self
+        return false
       end
 
       node.obj.try &.accept self
diff --git a/src/compiler/crystal/semantic/to_s.cr b/src/compiler/crystal/semantic/to_s.cr
index 8c91426bc379..13e8203efe1d 100644
--- a/src/compiler/crystal/semantic/to_s.cr
+++ b/src/compiler/crystal/semantic/to_s.cr
@@ -44,14 +44,17 @@ module Crystal
     def visit(node : Primitive)
       @str << "# primitive: "
       @str << node.name
+      false
     end
 
     def visit(node : MetaVar)
       @str << node.name
+      false
     end
 
     def visit(node : MetaMacroVar)
       @str << node.name
+      false
     end
 
     def visit(node : TypeFilteredNode)
diff --git a/src/compiler/crystal/semantic/top_level_visitor.cr b/src/compiler/crystal/semantic/top_level_visitor.cr
index 11c5707b19fa..9b7eedb2a876 100644
--- a/src/compiler/crystal/semantic/top_level_visitor.cr
+++ b/src/compiler/crystal/semantic/top_level_visitor.cr
@@ -598,6 +598,7 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor
       typed_def_type = lookup_type(node.type_spec)
       typed_def_type = check_allowed_in_lib node.type_spec, typed_def_type
       current_type.types[node.name] = TypeDefType.new @program, current_type, node.name, typed_def_type
+      false
     end
   end
 
diff --git a/src/compiler/crystal/semantic/type_guess_visitor.cr b/src/compiler/crystal/semantic/type_guess_visitor.cr
index 693cade2906b..612390db8e21 100644
--- a/src/compiler/crystal/semantic/type_guess_visitor.cr
+++ b/src/compiler/crystal/semantic/type_guess_visitor.cr
@@ -86,6 +86,7 @@ module Crystal
       end
 
       check_var_is_self(node)
+      false
     end
 
     def visit(node : UninitializedVar)
@@ -114,6 +115,7 @@ module Crystal
           # TODO: can this be reached?
         end
       end
+      false
     end
 
     def visit(node : Assign)
@@ -1353,6 +1355,7 @@ module Crystal
       if node.name == "self"
         @has_self = true
       end
+      false
     end
 
     def visit(node : ASTNode)
diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr
index f5086e346277..3ea146d15c3e 100644
--- a/src/compiler/crystal/syntax/to_s.cr
+++ b/src/compiler/crystal/syntax/to_s.cr
@@ -49,10 +49,12 @@ module Crystal
     end
 
     def visit(node : Nop)
+      false
     end
 
     def visit(node : BoolLiteral)
       @str << (node.value ? "true" : "false")
+      false
     end
 
     def visit(node : NumberLiteral)
@@ -62,6 +64,8 @@ module Crystal
         @str << '_'
         @str << node.kind.to_s
       end
+
+      false
     end
 
     def needs_suffix?(node : NumberLiteral)
@@ -83,10 +87,12 @@ module Crystal
 
     def visit(node : CharLiteral)
       node.value.inspect(@str)
+      false
     end
 
     def visit(node : SymbolLiteral)
       visit_symbol_literal_value node.value
+      false
     end
 
     def visit_symbol_literal_value(value : String)
@@ -100,6 +106,7 @@ module Crystal
 
     def visit(node : StringLiteral)
       node.value.inspect(@str)
+      false
     end
 
     def visit(node : StringInterpolation)
@@ -189,6 +196,7 @@ module Crystal
 
     def visit(node : NilLiteral)
       @str << "nil"
+      false
     end
 
     def visit(node : Expressions)
@@ -573,6 +581,7 @@ module Crystal
 
     def visit(node : Var)
       @str << node.name
+      false
     end
 
     def visit(node : ProcLiteral)
@@ -836,11 +845,13 @@ module Crystal
 
     def visit(node : Self)
       @str << "self"
+      false
     end
 
     def visit(node : Path)
       @str << "::" if node.global?
       node.names.join(@str, "::")
+      false
     end
 
     def visit(node : Generic)
@@ -907,6 +918,7 @@ module Crystal
 
     def visit(node : InstanceVar)
       @str << node.name
+      false
     end
 
     def visit(node : ReadInstanceVar)
@@ -918,6 +930,7 @@ module Crystal
 
     def visit(node : ClassVar)
       @str << node.name
+      false
     end
 
     def visit(node : Yield)
@@ -1105,6 +1118,7 @@ module Crystal
 
     def visit(node : Global)
       @str << node.name
+      false
     end
 
     def visit(node : LibDef)
@@ -1449,6 +1463,7 @@ module Crystal
 
     def visit(node : MagicConstant)
       @str << node.name
+      false
     end
 
     def visit(node : Asm)
diff --git a/src/compiler/crystal/tools/expand.cr b/src/compiler/crystal/tools/expand.cr
index 365fc2b3f865..792d60869bd3 100644
--- a/src/compiler/crystal/tools/expand.cr
+++ b/src/compiler/crystal/tools/expand.cr
@@ -144,6 +144,8 @@ module Crystal
         else
           contains_target(node)
         end
+      else
+        false
       end
     end
 
@@ -156,6 +158,8 @@ module Crystal
         else
           contains_target(node)
         end
+      else
+        false
       end
     end
 
diff --git a/src/compiler/crystal/tools/implementations.cr b/src/compiler/crystal/tools/implementations.cr
index f4f3a390f0eb..e2dbee001346 100644
--- a/src/compiler/crystal/tools/implementations.cr
+++ b/src/compiler/crystal/tools/implementations.cr
@@ -114,6 +114,7 @@ module Crystal
           @locations << target_def.location.not_nil!
         end
       end
+      false
     end
 
     def visit(node : Path)
@@ -123,6 +124,7 @@ module Crystal
       target.try &.locations.try &.each do |loc|
         @locations << loc
       end
+      false
     end
 
     def visit(node)
diff --git a/src/compiler/crystal/tools/print_types_visitor.cr b/src/compiler/crystal/tools/print_types_visitor.cr
index 6ffc533a4b0f..0744dc17febd 100644
--- a/src/compiler/crystal/tools/print_types_visitor.cr
+++ b/src/compiler/crystal/tools/print_types_visitor.cr
@@ -39,10 +39,12 @@ module Crystal
 
     def visit(node : Var)
       output_name node
+      false
     end
 
     def visit(node : Global)
       output_name node
+      false
     end
 
     def visit(node : TypeDeclaration)
@@ -50,6 +52,7 @@ module Crystal
       if var.is_a?(Var)
         output_name var
       end
+      false
     end
 
     def visit(node : UninitializedVar)
@@ -57,6 +60,7 @@ module Crystal
       if var.is_a?(Var)
         output_name var
       end
+      false
     end
 
     def output_name(node)