Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Refinements Broken from 9.1 to 9.2 #5288

Closed
fny opened this issue Aug 22, 2018 · 3 comments
Closed

Regression: Refinements Broken from 9.1 to 9.2 #5288

fny opened this issue Aug 22, 2018 · 3 comments

Comments

@fny
Copy link

@fny fny commented Aug 22, 2018

A Ruby library of mine, which provides refinements, was working as of JRuby 9.1.16.0. I recently added JRuby 9.2.0.0 and found that two tests which use refinements are now broken. Note all tests pass across all other Ruby versions, and I have not made any updates to the library.

1) Error:
RefinementsTest#test_xor:
NoMethodError: undefined method `xor' for "w\x12\xC5_\xD7\xB8\x06\xF3\xD4\x9Bl\x909\xF8(M":String
   /home/travis/build/fny/xorcist/test/refinements_test.rb:10:in `test_xor'
 2) Error:
RefinementsTest#test_xor_in_place:
NoMethodError: undefined method `xor!' for "String":String
   /home/travis/build/fny/xorcist/test/refinements_test.rb:20:in `block in test_xor_in_place'
   /home/travis/build/fny/xorcist/test/test_helper.rb:17:in `frozen_strings_dependent'
   /home/travis/build/fny/xorcist/test/refinements_test.rb:19:in `test_xor_in_place'

Full trace available here:
https://travis-ci.org/fny/xorcist/jobs/418932838

I've included the relevant code below as a convenience:

# test/refinements_test.rb:
require 'test_helper'

if RUBY_ENGINE != 'rbx' && RUBY_VERSION >= '2.0.0'
  require 'xorcist/refinements'

  class RefinementsTest < Minitest::Test
    using Xorcist::Refinements

    def test_xor
      assert_equal ZERO, X.xor(X)
      assert_equal ONE,  X.xor(INVX)
      assert_equal X,    X.xor(ZERO)
      assert_equal INVX, X.xor(ONE)
    end

    def test_xor_in_place
      a = "String"
      b = a
      frozen_strings_dependent {
        b.xor!(X)
        assert_equal(a, b)
      }
    end
  end
end

# lib/xorcist/refinements.rb
require 'xorcist/string_methods'

module Xorcist
  module Refinements
    refine String do
      include Xorcist::StringMethods
    end
  end
end

# lib/xorcist/string_methods.rb
module Xorcist
  module StringMethods
    def xor(other)
      Xorcist.xor(self, other)
    end

    def xor!(other)
      Xorcist.xor!(self, other)
    end
  end
end
@fny fny changed the title Regression Regression: Refinements Broken from 9.1 to 9.2 Aug 22, 2018
@enebo enebo added this to the JRuby 9.2.1.0 milestone Aug 23, 2018
@headius
Copy link
Member

@headius headius commented Aug 28, 2018

Thank you for the report!

@alexis779
Copy link
Contributor

@alexis779 alexis779 commented Nov 29, 2018

It seems Module#include is no longer supported in Module#refine block. It works when module functions are inline:

diff --git a/lib/xorcist/refinements.rb b/lib/xorcist/refinements.rb
index b6f1eb9..c472a92 100644
--- a/lib/xorcist/refinements.rb
+++ b/lib/xorcist/refinements.rb
@@ -3,7 +3,13 @@ require 'xorcist/string_methods'
 module Xorcist
   module Refinements
     refine String do
-      include Xorcist::StringMethods
+      def xor(other)
+        Xorcist.xor(self, other)
+      end
+
+      def xor!(other)
+        Xorcist.xor!(self, other)
+      end
     end
   end
 end
alexis779 pushed a commit to alexis779/jruby that referenced this issue Dec 1, 2018
@alexis779
Copy link
Contributor

@alexis779 alexis779 commented Dec 1, 2018

Your gem test-suite is successful with #5487

ruby -v
  jruby 9.2.5.0-SNAPSHOT (2.5.0) 2018-11-29 a225214 Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 +jit [darwin-x86_64]
gem install bundler
bundle install
bundle exec rake test
  8 runs, 19 assertions, 0 failures, 0 errors, 0 skips
@kares kares closed this in 1bf79da Dec 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants