Implementation of Module#prepend by LTe · Pull Request #1848 · rubinius/rubinius

LTe · 2012-08-03T12:49:32Z

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 · 2012-08-03T13:05:42Z

This pull request fails (merged e12da472 into df43311).

travisbot · 2012-08-03T13:07:25Z

This pull request fails (merged e641e34e into df43311).

travisbot · 2012-08-03T13:22:16Z

This pull request fails (merged 3524617 into df43311).

brixen · 2012-08-03T16:57:26Z

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.

Use ruby guard. Test only for ruby => 2.0

travisbot · 2012-08-09T09:21:24Z

This pull request fails (merged d6d2f3eb into df43311).

travisbot · 2012-08-09T09:37:41Z

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

Method attach_before should remember last parent. Without this we can use prepend only once.

Developer can implement that method and put the logic in method

travisbot · 2012-08-21T10:55:29Z

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

travisbot · 2012-08-21T12:03:49Z

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

LTe · 2013-07-27T16:46:11Z

@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 · 2013-07-27T16:55:42Z

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 · 2013-07-27T16:56:01Z

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

LTe added 2 commits August 3, 2012 14:30

Add simple spec for ruby prepend

48e6c1e

Add partial implementation of Module#prepend

3524617

LTe added 3 commits August 9, 2012 10:46

Add helpers for labeled modules

8fe17e5

Add fixtures for prepended modules

ea2fae5

Update specs for Module#prepend

36d282f

Use ruby guard. Test only for ruby => 2.0

LTe added 7 commits August 21, 2012 10:55

Add slot for superclass_root in module

84439a4

Update attach_before method.

af3cbed

Method attach_before should remember last parent. Without this we can use prepend only once.

Add spec for ancestors n the case of prepend

35b3311

Method ancestors should work find with #prepend

5a92f9c

Don't prepend module twice

eb0b369

Add empty #prepended method

59d0780

Developer can implement that method and put the logic in method

Update guard to 2.0

648a7e8

Do not load classes with prepend in ruby < 1.9.3

49e09d8

dbussink closed this in 9a8e2df Jul 26, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of Module#prepend#1848

Implementation of Module#prepend#1848
LTe wants to merge 13 commits intorubinius:masterfrom
LTe:prepend

LTe commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

brixen commented Aug 3, 2012

Uh oh!

travisbot commented Aug 9, 2012

Uh oh!

travisbot commented Aug 9, 2012

Uh oh!

travisbot commented Aug 21, 2012

Uh oh!

travisbot commented Aug 21, 2012

Uh oh!

LTe commented Jul 27, 2013

Uh oh!

dbussink commented Jul 27, 2013

Uh oh!

dbussink commented Jul 27, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

LTe commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

travisbot commented Aug 3, 2012

Uh oh!

brixen commented Aug 3, 2012

Uh oh!

travisbot commented Aug 9, 2012

Uh oh!

travisbot commented Aug 9, 2012

Uh oh!

travisbot commented Aug 21, 2012

Uh oh!

travisbot commented Aug 21, 2012

Uh oh!

LTe commented Jul 27, 2013

Uh oh!

dbussink commented Jul 27, 2013

Uh oh!

dbussink commented Jul 27, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants