Use a priority queue to select compilers

The existing case-statement with nested if-statements is gross and hard
to extend. Replacing it with a priority queue simplifies the logic and
makes it very easy to add new compilers to the fails_with system, which
we will likely want to do in the future.

3 files changed, 81 insertions, 75 deletions

diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb
index b4fbef99c..fb42c5e30 100644
--- a/Library/Homebrew/compilers.rb
+++ b/Library/Homebrew/compilers.rb
@@ -1,44 +1,14 @@
-class Compilers
-  include Enumerable
-
-  def initialize(*args)
-    @compilers = Array.new(*args)
-  end
-
-  def each(*args, &block)
-    @compilers.each(*args, &block)
-  end
-
-  def include?(cc)
-    cc = cc.name if cc.is_a? Compiler
-    @compilers.any? { |c| c.name == cc }
-  end
-
-  def <<(o)
-    @compilers << o
-    self
-  end
-end
-
-
-class Compiler
-  attr_reader :name, :build
-
-  def initialize name
-    @name = name
-    @build = case name
-    when :clang then MacOS.clang_build_version.to_i
-    when :llvm then MacOS.llvm_build_version.to_i
-    when :gcc then MacOS.gcc_42_build_version.to_i
+class Compiler < Struct.new(:name, :priority)
+  def build
+    case name
+    when :clang, :llvm
+      MacOS.send("#{name}_build_version")
+    when :gcc
+      MacOS.gcc_42_build_version
     end
   end
-
-  def ==(other)
-    @name.to_sym == other.to_sym
-  end
 end
 
-
 class CompilerFailure
   attr_reader :compiler
 
@@ -57,50 +27,49 @@ class CompilerFailure
   end
 end
 
+class CompilerQueue
+  def initialize
+    @array = []
+  end
 
-# CompilerSelector is used to process a formula's CompilerFailures.
-# If no viable compilers are available, ENV.compiler is left as-is.
-class CompilerSelector
-  NAMES = { :clang => "Clang", :gcc => "GCC", :llvm => "LLVM" }
+  def <<(o)
+    @array << o
+    self
+  end
+
+  def pop
+    @array.delete(@array.max { |a, b| a.priority <=> b.priority })
+  end
 
-  def initialize f
+  def empty?
+    @array.empty?
+  end
+end
+
+class CompilerSelector
+  def initialize(f, old_compiler=ENV.compiler)
     @f = f
-    @old_compiler = ENV.compiler
-    @compilers = Compilers.new
-    @compilers << Compiler.new(:clang) if MacOS.clang_build_version
-    @compilers << Compiler.new(:llvm) if MacOS.llvm_build_version
-    @compilers << Compiler.new(:gcc) if MacOS.gcc_42_build_version
+    @old_compiler = old_compiler
+    @compilers = CompilerQueue.new
+    %w{clang llvm gcc}.map(&:to_sym).each do |cc|
+      @compilers << Compiler.new(cc, priority_for(cc))
+    end
   end
 
   def select_compiler
-    # @compilers is our list of available compilers. If @f declares a
-    # failure with compiler foo, then we remove foo from the list if
-    # the failing build is >= the currently installed version of foo.
-    @compilers = @compilers.reject do |cc|
-      failure = @f.fails_with? cc
-      failure && failure.build >= cc.build
-    end
+    begin
+      cc = @compilers.pop
+    end while @f.fails_with?(cc)
+    ENV.send(cc.name) unless cc.nil?
+  end
 
-    return if @compilers.empty? or @compilers.include? ENV.compiler
+  private
 
-    ENV.send case ENV.compiler
-    when :clang
-      if @compilers.include? :llvm then :llvm
-      elsif @compilers.include? :gcc then :gcc
-      else ENV.compiler
-      end
-    when :llvm
-      if @compilers.include? :clang and MacOS.clang_build_version >= 211 then :clang
-      elsif @compilers.include? :gcc then :gcc
-      elsif @compilers.include? :clang then :clang
-      else ENV.compiler
-      end
-    when :gcc
-      if @compilers.include? :clang and MacOS.clang_build_version >= 211 then :clang
-      elsif @compilers.include? :llvm then :llvm
-      elsif @compilers.include? :clang then :clang
-      else ENV.compiler
-      end
+  def priority_for(cc)
+    case cc
+    when :clang then MacOS.clang_build_version >= 211 ? 3 : 0.5
+    when :llvm  then 2
+    when :gcc   then 1
     end
   end
 end
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index 7ba0cd01e..77b1fabdb 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -198,9 +198,8 @@ class Formula
   end
 
   def fails_with? cc
-    return false if self.class.cc_failures.nil?
     cc = Compiler.new(cc) unless cc.is_a? Compiler
-    self.class.cc_failures.find do |failure|
+    (self.class.cc_failures || []).any? do |failure|
       failure.compiler == cc.name && failure.build >= cc.build
     end
   end
diff --git a/Library/Homebrew/test/test_compiler_queue.rb b/Library/Homebrew/test/test_compiler_queue.rb
new file mode 100644
index 000000000..6dee1ab37
--- /dev/null
+++ b/Library/Homebrew/test/test_compiler_queue.rb
@@ -0,0 +1,38 @@
+require 'testing_env'
+require 'compilers'
+
+class CompilerQueueTests < Test::Unit::TestCase
+  FakeCompiler = Struct.new(:name, :priority)
+
+  def setup
+    @q = CompilerQueue.new
+  end
+
+  def test_shovel_returns_self
+    assert_same @q, (@q << Object.new)
+  end
+
+  def test_empty
+    assert @q.empty?
+  end
+
+  def test_queues_items
+    a = FakeCompiler.new(:foo, 0)
+    b = FakeCompiler.new(:bar, 0)
+    @q << a << b
+    assert_equal a, @q.pop
+    assert_equal b, @q.pop
+    assert_nil @q.pop
+  end
+
+  def test_pops_items_by_priority
+    a = FakeCompiler.new(:foo, 0)
+    b = FakeCompiler.new(:bar, 0.5)
+    c = FakeCompiler.new(:baz, 1)
+    @q << a << b << c
+    assert_equal c, @q.pop
+    assert_equal b, @q.pop
+    assert_equal a, @q.pop
+    assert_nil @q.pop
+  end
+end