Implementation of Module#prepend #1848

Closed
wants to merge 13 commits into
from

Conversation

Projects
None yet
4 participants
@LTe
Contributor

LTe commented Aug 3, 2012

Ruby 2.0.0

module M; end

class C
  include M
end

C.ancestors # => [C, M, Object, Kernel, BasicObject] 

class C1
  prepend M
end

C1.ancestors # => [M, C1, Object, Kernel, BasicObject] 

This is only partial implementation of this feature. I don't know how change superclass chain. I found in Rubinius code. In this case we need create root for IncludedModule

  void test_object_class_with_superclass_chain() {
    Module* mod = Module::create(state);
    Class* cls = Class::create(state, G(object));
    Object* obj = state->new_object<Object>(cls);

    /* This should be functionally correct but not actually the
     * way a superclass chain is implemented. However, it doesn't
     * require that we create a root for IncludedModule.
     */
    Module* m = cls->superclass();
    cls->superclass(state, mod);
    mod->superclass(state, m);

    TS_ASSERT_EQUALS(cls, obj->class_object(state));
  }

In MRI implementation each RClass have origin field. On start origin points to itself. But after prepend interpreter allocate memory for new class and create another.

origin = class_alloc(T_ICLASS, klass);
RCLASS_SUPER(origin) = RCLASS_SUPER(klass);
RCLASS_SUPER(klass) = origin;
RCLASS_ORIGIN(klass) = origin;
RCLASS_M_TBL(origin) = RCLASS_M_TBL(klass);
RCLASS_M_TBL(klass) = 0;

So klass is still on top but have new origin. Little tricky for me :)

Someone can help me with Module#prepend implementation? Maybe Rubinius internals are not ready for prepend. Any advice appreciated.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 3, 2012

This pull request fails (merged e12da472 into df43311).

This pull request fails (merged e12da472 into df43311).

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 3, 2012

This pull request fails (merged e641e34e into df43311).

This pull request fails (merged e641e34e into df43311).

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 3, 2012

This pull request fails (merged 3524617 into df43311).

This pull request fails (merged 3524617 into df43311).

@brixen

This comment has been minimized.

Show comment Hide comment
@brixen

brixen Aug 3, 2012

Member

Your specs need a proper version guard for 2.0 so they don't fail on 1.8 and 1.9.

I don't think we need prepend in kernel/alpha.rb as I don't expect us to be using it in Rubinius itself, so it doesn't need to be available to load the kernel.

The rest I haven't looked into yet.

Member

brixen commented Aug 3, 2012

Your specs need a proper version guard for 2.0 so they don't fail on 1.8 and 1.9.

I don't think we need prepend in kernel/alpha.rb as I don't expect us to be using it in Rubinius itself, so it doesn't need to be available to load the kernel.

The rest I haven't looked into yet.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 9, 2012

This pull request fails (merged d6d2f3eb into df43311).

This pull request fails (merged d6d2f3eb into df43311).

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 9, 2012

This pull request fails (merged 36d282f into d47f143).

This pull request fails (merged 36d282f into d47f143).

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 21, 2012

This pull request fails (merged 648a7e8 into d47f143).

This pull request fails (merged 648a7e8 into d47f143).

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 21, 2012

This pull request fails (merged 49e09d8 into d47f143).

This pull request fails (merged 49e09d8 into d47f143).

@dbussink dbussink closed this in 9a8e2df Jul 26, 2013

@LTe

This comment has been minimized.

Show comment Hide comment
@LTe

LTe Jul 27, 2013

Contributor

@dbussink very nice implementation of Module#prepend. I used my specs to validate your solution

module ModuleSpecs
  if RUBY_VERSION > "1.9.3"
    module PrependModules
      module M0
        def m1; [:M0] end
      end
      module M1
        def m1; [:M1, *super] end
      end
      module M2
        def m1; [:M2, *super] end
      end
      M3 = Module.new do
        def m1; [:M3, *super] end
      end
      module M4
        def m1; [:M4, *super] end
      end
      class C
        def m1; end
      end
      class C0 < C
        include M0
        prepend M1
        def m1; [:C0, *super] end
      end
      class C1 < C0
        prepend M2, M3
        include M4
        def m1; [:C1, *super] end
      end
    end

    module ModuleToPrepend
      def m
        result = super
        [:m, result]
      end
    end

    class ClassToPrepend
      prepend ModuleToPrepend
      def m
        :c
      end
    end
  end
end

class Object
  def labeled_module(name, &block)
    Module.new do
      singleton_class.class_eval {define_method(:to_s) {name}}
      class_eval(&block) if block
    end
  end

  def labeled_class(name, superclass = Object, &block)
    Class.new(superclass) do
      singleton_class.class_eval {define_method(:to_s) {name}}
      class_eval(&block) if block
    end
  end
end

ruby_version_is "2.0" do
  describe "Module#prepend" do
    it "prepends modules in proper sequence" do
      obj = ModuleSpecs::PrependModules::C0.new
      obj.m1.should == [:M1,:C0,:M0]

      obj = ModuleSpecs::PrependModules::C1.new
      obj.m1.should == [:M2,:M3,:C1,:M4,:M1,:C0,:M0]
    end

    it "returns proper prepend module ancestors" do
      m0 = labeled_module("m0") {def x; [:m0, *super] end}
      m1 = labeled_module("m1") {def x; [:m1, *super] end; prepend m0}
      m2 = labeled_module("m2") {def x; [:m2, *super] end; prepend m1}
      c0 = labeled_class("c0") {def x; [:c0] end}
      c1 = labeled_class("c1") {def x; [:c1] end; prepend m2}
      c2 = labeled_class("c2", c0) {def x; [:c2, *super] end; include m2}

      m1.ancestors.should == [m0, m1]

      c1.ancestors[0, 4].should == [m0, m1, m2, c1]
      m2.ancestors.should == [m0, m1, m2]
      c1.new.x.should == [:m0, :m1, :m2, :c1]
      c2.ancestors[0, 5].should == [c2, m0, m1, m2, c0]
      c2.new.x.should == [:c2, :m0, :m1, :m2, :c0]
    end

    it "updates ancestors after prepend" do
      m   = Module.new
      m1  = Module.new
      c   = Class.new { prepend m }
      c1  = Class.new(c)

      c1.ancestors.should include(m)
      c1.ancestors.should_not include(m1)

      c.send(:prepend, m1)
      c1.ancestors.should include(m1)
    end
  end
end

In result

1)
Module#prepend prepends modules in proper sequence ERROR
SystemStackError: SystemStackError
                                   ModuleSpecs::PrependModules::C0#m1 at spec/ruby/core/module/fixtures/classes.rb:433 (6192 times)
  ModuleSpecs::PrependModules::M1(ModuleSpecs::PrependModules::C0)#m1 at spec/ruby/core/module/fixtures/classes.rb:416
                                             { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:24
                                    BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                                        { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                                                           Array#each at kernel/bootstrap/array.rb:68
                                               Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                                                Integer(Fixnum)#times at kernel/common/integer.rb:83
                                                           Array#each at kernel/bootstrap/array.rb:68
                                             { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                                                    Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                                                          Kernel.load at kernel/common/kernel.rb:588
                                    BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                                                           Array#each at kernel/bootstrap/array.rb:68
                                     Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
                                     Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
                                              Rubinius::Loader#script at kernel/loader.rb:645
                                                Rubinius::Loader#main at kernel/loader.rb:844

2)
Module#prepend returns proper prepend module ancestors FAILED
Expected [m2, m0, m1, c1]
 to equal [m0, m1, m2, c1]

           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:40
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
      { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                         Array#each at kernel/bootstrap/array.rb:68
             Enumerable(Array)#all? at kernel/common/enumerable.rb:102
              Integer(Fixnum)#times at kernel/common/integer.rb:83
                         Array#each at kernel/bootstrap/array.rb:68
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                  Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                        Kernel.load at kernel/common/kernel.rb:588
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                         Array#each at kernel/bootstrap/array.rb:68
   Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
   Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
            Rubinius::Loader#script at kernel/loader.rb:645
              Rubinius::Loader#main at kernel/loader.rb:844

3)
Module#prepend updates ancestors after prepend FAILED
Expected [#<Class:0x2438>, #<Module:0x243c>, #<Class:0x2440>, Object, PP::ObjectMixin, Kernel, BasicObject]
to include #<Module:0x2444>
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:57
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
      { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                         Array#each at kernel/bootstrap/array.rb:68
             Enumerable(Array)#all? at kernel/common/enumerable.rb:102
              Integer(Fixnum)#times at kernel/common/integer.rb:83
                         Array#each at kernel/bootstrap/array.rb:68
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                  Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                        Kernel.load at kernel/common/kernel.rb:588
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                         Array#each at kernel/bootstrap/array.rb:68
   Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
   Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
            Rubinius::Loader#script at kernel/loader.rb:645
              Rubinius::Loader#main at kernel/loader.rb:844

Finished in 0.032570 seconds

1 file, 3 examples, 5 expectations, 2 failures, 1 error
Contributor

LTe commented Jul 27, 2013

@dbussink very nice implementation of Module#prepend. I used my specs to validate your solution

module ModuleSpecs
  if RUBY_VERSION > "1.9.3"
    module PrependModules
      module M0
        def m1; [:M0] end
      end
      module M1
        def m1; [:M1, *super] end
      end
      module M2
        def m1; [:M2, *super] end
      end
      M3 = Module.new do
        def m1; [:M3, *super] end
      end
      module M4
        def m1; [:M4, *super] end
      end
      class C
        def m1; end
      end
      class C0 < C
        include M0
        prepend M1
        def m1; [:C0, *super] end
      end
      class C1 < C0
        prepend M2, M3
        include M4
        def m1; [:C1, *super] end
      end
    end

    module ModuleToPrepend
      def m
        result = super
        [:m, result]
      end
    end

    class ClassToPrepend
      prepend ModuleToPrepend
      def m
        :c
      end
    end
  end
end

class Object
  def labeled_module(name, &block)
    Module.new do
      singleton_class.class_eval {define_method(:to_s) {name}}
      class_eval(&block) if block
    end
  end

  def labeled_class(name, superclass = Object, &block)
    Class.new(superclass) do
      singleton_class.class_eval {define_method(:to_s) {name}}
      class_eval(&block) if block
    end
  end
end

ruby_version_is "2.0" do
  describe "Module#prepend" do
    it "prepends modules in proper sequence" do
      obj = ModuleSpecs::PrependModules::C0.new
      obj.m1.should == [:M1,:C0,:M0]

      obj = ModuleSpecs::PrependModules::C1.new
      obj.m1.should == [:M2,:M3,:C1,:M4,:M1,:C0,:M0]
    end

    it "returns proper prepend module ancestors" do
      m0 = labeled_module("m0") {def x; [:m0, *super] end}
      m1 = labeled_module("m1") {def x; [:m1, *super] end; prepend m0}
      m2 = labeled_module("m2") {def x; [:m2, *super] end; prepend m1}
      c0 = labeled_class("c0") {def x; [:c0] end}
      c1 = labeled_class("c1") {def x; [:c1] end; prepend m2}
      c2 = labeled_class("c2", c0) {def x; [:c2, *super] end; include m2}

      m1.ancestors.should == [m0, m1]

      c1.ancestors[0, 4].should == [m0, m1, m2, c1]
      m2.ancestors.should == [m0, m1, m2]
      c1.new.x.should == [:m0, :m1, :m2, :c1]
      c2.ancestors[0, 5].should == [c2, m0, m1, m2, c0]
      c2.new.x.should == [:c2, :m0, :m1, :m2, :c0]
    end

    it "updates ancestors after prepend" do
      m   = Module.new
      m1  = Module.new
      c   = Class.new { prepend m }
      c1  = Class.new(c)

      c1.ancestors.should include(m)
      c1.ancestors.should_not include(m1)

      c.send(:prepend, m1)
      c1.ancestors.should include(m1)
    end
  end
end

In result

1)
Module#prepend prepends modules in proper sequence ERROR
SystemStackError: SystemStackError
                                   ModuleSpecs::PrependModules::C0#m1 at spec/ruby/core/module/fixtures/classes.rb:433 (6192 times)
  ModuleSpecs::PrependModules::M1(ModuleSpecs::PrependModules::C0)#m1 at spec/ruby/core/module/fixtures/classes.rb:416
                                             { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:24
                                    BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                                        { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                                                           Array#each at kernel/bootstrap/array.rb:68
                                               Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                                                Integer(Fixnum)#times at kernel/common/integer.rb:83
                                                           Array#each at kernel/bootstrap/array.rb:68
                                             { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                                                    Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                                                          Kernel.load at kernel/common/kernel.rb:588
                                    BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                                                           Array#each at kernel/bootstrap/array.rb:68
                                     Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
                                     Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
                                              Rubinius::Loader#script at kernel/loader.rb:645
                                                Rubinius::Loader#main at kernel/loader.rb:844

2)
Module#prepend returns proper prepend module ancestors FAILED
Expected [m2, m0, m1, c1]
 to equal [m0, m1, m2, c1]

           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:40
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
      { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                         Array#each at kernel/bootstrap/array.rb:68
             Enumerable(Array)#all? at kernel/common/enumerable.rb:102
              Integer(Fixnum)#times at kernel/common/integer.rb:83
                         Array#each at kernel/bootstrap/array.rb:68
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                  Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                        Kernel.load at kernel/common/kernel.rb:588
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                         Array#each at kernel/bootstrap/array.rb:68
   Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
   Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
            Rubinius::Loader#script at kernel/loader.rb:645
              Rubinius::Loader#main at kernel/loader.rb:844

3)
Module#prepend updates ancestors after prepend FAILED
Expected [#<Class:0x2438>, #<Module:0x243c>, #<Class:0x2440>, Object, PP::ObjectMixin, Kernel, BasicObject]
to include #<Module:0x2444>
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:57
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
      { } in Enumerable(Array)#all? at kernel/common/enumerable.rb:102
                         Array#each at kernel/bootstrap/array.rb:68
             Enumerable(Array)#all? at kernel/common/enumerable.rb:102
              Integer(Fixnum)#times at kernel/common/integer.rb:83
                         Array#each at kernel/bootstrap/array.rb:68
           { } in Object#__script__ at spec/ruby/core/module/prepend_spec.rb:21
                  Object#__script__ at spec/ruby/core/module/prepend_spec.rb:20
                        Kernel.load at kernel/common/kernel.rb:588
  BasicObject(Object)#instance_eval at kernel/common/eval19.rb:45
                         Array#each at kernel/bootstrap/array.rb:68
   Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:68
   Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:119
            Rubinius::Loader#script at kernel/loader.rb:645
              Rubinius::Loader#main at kernel/loader.rb:844

Finished in 0.032570 seconds

1 file, 3 examples, 5 expectations, 2 failures, 1 error
@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Jul 27, 2013

Member

Can you open an issue for that / open a pull request with the additional specs in it? If it's just a commit comment, we're going to forget about it.

Member

dbussink commented Jul 27, 2013

Can you open an issue for that / open a pull request with the additional specs in it? If it's just a commit comment, we're going to forget about it.

@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Jul 27, 2013

Member

Probably better to start with a new pull request / issue than trying to rework this one.

Member

dbussink commented Jul 27, 2013

Probably better to start with a new pull request / issue than trying to rework this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment