Compiler: restrict else parr of an if with ||. Fixes #3266. Fixes #3412

crystal-lang · Oct 12, 2016 · 1119f02 · 1119f02
1 parent a33c0c2
commit 1119f02
Showing 3 changed files with 136 additions and 1 deletion.
diff --git a/spec/compiler/semantic/if_spec.cr b/spec/compiler/semantic/if_spec.cr
@@ -239,4 +239,102 @@ describe "Semantic: if" do
       test
       ), inject_primitives: false) { int32 }
   end
+
+  it "restricts || else (1) (#3266)" do
+    assert_type(%(
+      a = 1 || nil
+      b = 1 || nil
+      if !a || !b
+        {1, 2}
+      else
+        {a, b}
+      end
+      ), inject_primitives: false) { tuple_of([int32, int32]) }
+  end
+
+  it "restricts || else (2) (#3266)" do
+    assert_type(%(
+      a = 1 || nil
+      if !a || 1
+        1
+      else
+        a
+      end
+      ), inject_primitives: false) { int32 }
+  end
+
+  it "restricts || else (3) (#3266)" do
+    assert_type(%(
+      a = 1 || nil
+      if 1 || !a
+        1
+      else
+        a
+      end
+      ), inject_primitives: false) { int32 }
+  end
+
+  it "doesn't restrict || else in sub && (right)" do
+    assert_type(%(
+      def foo
+        a = 1 || nil
+
+        if false || (!a && false)
+          return 1
+        end
+
+        a
+      end
+
+      foo
+      )) { nilable int32 }
+  end
+
+  it "doesn't restrict || else in sub && (left)" do
+    assert_type(%(
+      def foo
+        a = 1 || nil
+
+        if (!a && false) || false
+          return 1
+        end
+
+        a
+      end
+
+      foo
+      )) { nilable int32 }
+  end
+
+  it "doesn't restrict || else in sub || (right)" do
+    assert_type(%(
+      def foo
+        a = 1 || nil
+
+        if false || (!a || false)
+          return 1
+        end
+
+        a
+      end
+
+      foo
+      )) { nilable int32 }
+  end
+
+  it "doesn't restrict || else in sub || (left)" do
+    assert_type(%(
+      def foo
+        a = 1 || nil
+
+        if (!a || false) || false
+          return 1
+        end
+
+        a
+      end
+
+      foo
+      )) { nilable int32 }
+  end
 end
diff --git a/src/compiler/crystal/semantic/filters.cr b/src/compiler/crystal/semantic/filters.cr
@@ -340,5 +340,11 @@ module Crystal
       end
       filters
     end
+
+    # Returns true if this filter is only applied to
+    # a temporary variable created by the compiler
+    def temp_var?
+      @filters.size == 1 && @filters.first_key.starts_with?("__temp_")
+    end
   end
 end
diff --git a/src/compiler/crystal/semantic/main_visitor.cr b/src/compiler/crystal/semantic/main_visitor.cr
@@ -72,6 +72,13 @@ module Crystal
     @file_module : FileModule?
     @while_vars : MetaVars?
 
+    # Separate type filters for an `a || b` expression.
+    # We need these to filter types on an else branch of an
+    # if that has an or expression, using boolean logic:
+    # `!(a || b)` is `!a && !b`
+    @or_left_type_filters : TypeFilters?
+    @or_right_type_filters : TypeFilters?
+
     def initialize(program, vars = MetaVars.new, @typed_def = nil, meta_vars = nil)
       super(program, vars)
       @while_stack = [] of While
@@ -103,6 +110,8 @@ module Crystal
 
     def visit_any(node)
       @unreachable = false
+      @or_left_type_filters = nil
+      @or_right_type_filters = nil
       super
     end
 
@@ -1684,6 +1693,8 @@ module Crystal
         node.cond.accept self
       end
 
+      or_left_type_filters = @or_left_type_filters
+      or_right_type_filters = @or_right_type_filters
       cond_type_filters = @type_filters
       cond_vars = @vars
 
@@ -1709,9 +1720,27 @@ module Crystal
       # block is when the condition is a Var (in the else it must be
       # nil), IsA (in the else it's not that type), RespondsTo
       # (in the else it doesn't respond to that message) or Not.
-      case node.cond
+      case cond = node.cond
       when Var, IsA, RespondsTo, Not
         filter_vars cond_type_filters, &.not
+      when Or
+        # Try to apply boolean logic: `!(a || b)` is `!a && !b`
+
+        #  We can't deduce anything for sub && or || expressions
+        or_left_type_filters = nil if cond.left.is_a?(And) || cond.left.is_a?(Or)
+        or_right_type_filters = nil if cond.right.is_a?(And) || cond.right.is_a?(Or)
+
+        # No need to deduce anything for temp vars created by the compiler (won't be used by a user)
+        or_left_type_filters = nil if or_left_type_filters && or_left_type_filters.temp_var?
+
+        if or_left_type_filters && or_right_type_filters
+          filters = TypeFilters.and(or_left_type_filters.not, or_right_type_filters.not)
+          filter_vars filters
+        elsif or_left_type_filters
+          filter_vars or_left_type_filters.not
+        elsif or_right_type_filters
+          filter_vars or_right_type_filters.not
+        end
       end
 
       before_else_vars = @vars.dup
@@ -1729,6 +1758,8 @@ module Crystal
         when .and?
           @type_filters = TypeFilters.and(cond_type_filters, then_type_filters, else_type_filters)
         when .or?
+          @or_left_type_filters = or_left_type_filters = then_type_filters
+          @or_right_type_filters = or_right_type_filters = else_type_filters
           @type_filters = TypeFilters.or(cond_type_filters, then_type_filters, else_type_filters)
         end
       end