Make some changes suggested by Flay

Author: chrisseaton

lib/truffle/bigdecimal.rb

@@ -101,42 +101,36 @@ def coerce(other)
     [BigDecimal(other, 20), self]
   end
 
-  # TODO (pitr 28-may-2015): compare with pure Java versions
   def >(other)
-    unless comp = (self <=> other)
-      return false if nan? || (BigDecimal === other && other.nan?)
-      raise ArgumentError, "comparison of #{self.class} with #{other.class}"
-    end
-
-    comp > 0
+    comp_helper(other, :>)
   end
 
   def >=(other)
-    unless comp = (self <=> other)
-      return false if nan? || (BigDecimal === other && other.nan?)
-      raise ArgumentError, "comparison of #{self.class} with #{other.class}"
-    end
-
-    comp >= 0
+    comp_helper(other, :>=)
   end
 
   def <(other)
-    unless comp = (self <=> other)
-      return false if nan? || (BigDecimal === other && other.nan?)
-      raise ArgumentError, "comparison of #{self.class} with #{other.class}"
-    end
-
-    comp < 0
+    comp_helper(other, :<)
   end
 
   def <=(other)
-    unless comp = (self <=> other)
-      return false if nan? || (BigDecimal === other && other.nan?)
-      raise ArgumentError, "comparison of #{self.class} with #{other.class}"
-    end
+    comp_helper(other, :<=)
+  end
 
-    comp <= 0
+  def comp_helper(other, op)
+    comp = (self <=> other)
+    if comp
+      # cmp >= 0, comp < 0, etc
+      comp.send(op, 0)
+    else
+      if nan? || (BigDecimal === other && other.nan?)
+        return false
+      else
+        raise ArgumentError, "comparison of #{self.class} with #{other.class}"
+      end
+    end
   end
+  private :comp_helper
 
   def nonzero?
     zero? ? nil : self

lib/truffle/socket/addrinfo.rb

@@ -338,34 +338,31 @@ def ipv6_sitelocal?
   end
 
   def ipv6_mc_global?
-    return false unless ipv6?
-    bytes = Truffle::Socket::Foreign.ip_to_bytes(afamily, ip_address)
-    bytes[0] == 0xff && bytes[1] & 0xf == 0xe
+    ipv6_mc_flag?(0xe)
   end
 
   def ipv6_mc_linklocal?
-    return false unless ipv6?
-    bytes = Truffle::Socket::Foreign.ip_to_bytes(afamily, ip_address)
-    bytes[0] == 0xff && bytes[1] & 0xf == 0x2
+    ipv6_mc_flag?(0x2)
   end
 
   def ipv6_mc_nodelocal?
-    return false unless ipv6?
-    bytes = Truffle::Socket::Foreign.ip_to_bytes(afamily, ip_address)
-    bytes[0] == 0xff && bytes[1] & 0xf == 0x1
+    ipv6_mc_flag?(0x1)
   end
 
   def ipv6_mc_orglocal?
-    return false unless ipv6?
-    bytes = Truffle::Socket::Foreign.ip_to_bytes(afamily, ip_address)
-    bytes[0] == 0xff && bytes[1] & 0xf == 0x8
+    ipv6_mc_flag?(0x8)
   end
 
   def ipv6_mc_sitelocal?
+    ipv6_mc_flag?(0x5)
+  end
+
+  def ipv6_mc_flag?(value)
     return false unless ipv6?
     bytes = Truffle::Socket::Foreign.ip_to_bytes(afamily, ip_address)
-    bytes[0] == 0xff && bytes[1] & 0xf == 0x5
+    bytes[0] == 0xff && bytes[1] & 0xf == value
   end
+  private :ipv6_mc_flag?
 
   def ipv6_to_ipv4
     return unless ipv6?

src/main/ruby/core/enumerable.rb

@@ -672,24 +672,7 @@ def first(n=undefined)
 
   def min(n = undefined, &block)
     return min_n(n, &block) if !undefined.equal?(n) && !n.nil?
-    min = undefined
-    each do
-      o = Truffle.single_block_arg
-      if undefined.equal? min
-        min = o
-      else
-        comp = block_given? ? yield(o, min) : o <=> min
-        unless comp
-          raise ArgumentError, "comparison of #{o.class} with #{min} failed"
-        end
-
-        if Comparable.compare_int(comp) < 0
-          min = o
-        end
-      end
-    end
-
-    undefined.equal?(min) ? nil : min
+    min_max(-1, &block)
   end
 
   def min_n(n, &block)
@@ -702,33 +685,38 @@ def min_n(n, &block)
 
   def max(n = undefined, &block)
     return max_n(n, &block) if !undefined.equal?(n) && !n.nil?
-    max = undefined
+    min_max(+1, &block)
+  end
+
+  def max_n(n, &block)
+    raise ArgumentError, "negative size #{n}" if n < 0
+    return [] if n == 0
+
+    self.sort(&block).last(n).reverse
+  end
+  private :max_n
+
+  def min_max(relative)
+    chosen = undefined
     each do
       o = Truffle.single_block_arg
-      if undefined.equal? max
-        max = o
+      if undefined.equal? chosen
+        chosen = o
       else
-        comp = block_given? ? yield(o, max) : o <=> max
+        comp = block_given? ? yield(o, chosen) : o <=> chosen
         unless comp
-          raise ArgumentError, "comparison of #{o.class} with #{max} failed"
+          raise ArgumentError, "comparison of #{o.class} with #{chosen} failed"
         end
-
-        if Comparable.compare_int(comp) > 0
-          max = o
+        
+        if (Comparable.compare_int(comp) <=> 0) == relative
+          chosen = o
         end
       end
     end
 
-    undefined.equal?(max) ? nil : max
-  end
-
-  def max_n(n, &block)
-    raise ArgumentError, "negative size #{n}" if n < 0
-    return [] if n == 0
-
-    self.sort(&block).last(n).reverse
+    undefined.equal?(chosen) ? nil : chosen
   end
-  private :max_n
+  private :min_max
 
   private def max_by_n(n, &block)
     n = Truffle::Type.rb_num2long(n)

src/main/ruby/core/file.rb

@@ -203,20 +203,23 @@ def self.basename(path, ext=undefined)
     path
   end
 
+  def self.query_stat_mode(path)
+    path = Truffle::Type.coerce_to_path(path)
+    Truffle::POSIX.truffleposix_stat_mode(path)
+  end
+
+  private_class_method :query_stat_mode
+
   ##
   # Returns true if the named file is a block device.
   def self.blockdev?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.blockdev?(mode)
+    Truffle::StatOperations.blockdev?(query_stat_mode(path))
   end
 
   ##
   # Returns true if the named file is a character device.
   def self.chardev?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.chardev?(mode)
+    Truffle::StatOperations.chardev?(query_stat_mode(path))
   end
 
   ##
@@ -581,9 +584,7 @@ def self.extname(path)
   ##
   # Returns true if the named file exists and is a regular file.
   def self.file?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.file?(mode)
+    Truffle::StatOperations.file?(query_stat_mode(path))
   end
 
   def self.braces(pattern, flags=0, patterns=[])
@@ -890,9 +891,7 @@ def self.path(obj)
   ##
   # Returns true if the named file is a pipe.
   def self.pipe?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.pipe?(mode)
+    Truffle::StatOperations.pipe?(query_stat_mode(path))
   end
 
   ##
@@ -1033,9 +1032,7 @@ def self.size?(io_or_path)
   ##
   # Returns true if the named file is a socket.
   def self.socket?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.socket?(mode)
+    Truffle::StatOperations.socket?(query_stat_mode(path))
   end
 
   ##
@@ -1155,17 +1152,11 @@ def self.utime(atime, mtime, *paths)
   end
 
   def self.world_readable?(path)
-    path = Truffle::Type.coerce_to_path path
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-
-    Truffle::StatOperations.world_readable?(mode)
+    Truffle::StatOperations.world_readable?(query_stat_mode(path))
   end
 
   def self.world_writable?(path)
-    path = Truffle::Type.coerce_to_path path
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-
-    Truffle::StatOperations.world_writable?(mode)
+    Truffle::StatOperations.world_writable?(query_stat_mode(path))
   end
 
   ##
@@ -1208,25 +1199,19 @@ def self.owned?(file_name)
   ##
   # Returns true if the named file has the setgid bit set.
   def self.setgid?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.setgid?(mode)
+    Truffle::StatOperations.setgid?(query_stat_mode(path))
   end
 
   ##
   # Returns true if the named file has the setuid bit set.
   def self.setuid?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.setuid?(mode)
+    Truffle::StatOperations.setuid?(query_stat_mode(path))
   end
 
   ##
   # Returns true if the named file has the sticky bit set.
   def self.sticky?(path)
-    path = Truffle::Type.coerce_to_path(path)
-    mode = Truffle::POSIX.truffleposix_stat_mode(path)
-    Truffle::StatOperations.sticky?(mode)
+    Truffle::StatOperations.sticky?(query_stat_mode(path))
   end
 
   class << self

src/main/ruby/core/hash.rb

@@ -115,39 +115,19 @@ def <=(other)
     self.class.contains_all_internal(self, other)
   end
 
-
   def ==(other)
-    return true if self.equal? other
-    unless other.kind_of? Hash
-      return false unless other.respond_to? :to_hash
-      return other == self
-    end
-
-    return false unless other.size == size
-
-    Thread.detect_recursion self, other do
-      each_pair do |key, value|
-        other_value = other._get_or_undefined(key)
-
-        # Other doesn't even have this key
-        return false if undefined.equal?(other_value)
-
-        # Order of the comparison matters! We must compare our value with
-        # the other Hash's value and not the other way around.
-        unless Truffle::Type.object_equal(value, other_value) or value == other_value
-          return false
-        end
-      end
-    end
-    true
+    eql_op(:==, other)
   end
 
   def eql?(other)
-    # Just like ==, but uses eql? to compare values.
+    eql_op(:eql?, other)
+  end
+
+  def eql_op(op, other)
     return true if self.equal? other
     unless other.kind_of? Hash
       return false unless other.respond_to? :to_hash
-      return other.eql?(self)
+      return other.send(op, self)
     end
 
     return false unless other.size == size
@@ -161,13 +141,14 @@ def eql?(other)
 
         # Order of the comparison matters! We must compare our value with
         # the other Hash's value and not the other way around.
-        unless Truffle::Type.object_equal(value, other_value) or value.eql?(other_value)
+        unless Truffle::Type.object_equal(value, other_value) or value.send(op, other_value)
           return false
         end
       end
     end
     true
   end
+  private :eql_op
 
   def >(other)
     other = Truffle::Type.coerce_to(other, Hash, :to_hash)