Skip to content

Commit

Permalink
Don't duplicate Proc objects if not needed
Browse files Browse the repository at this point in the history
Currently, when its receiver is a subclass of Proc, Proc::from_env() duplicates
passed objects, even if not needed. The case is that passing instances of the
same subclass of Proc:

    class MyProc < Proc
    end
    prc = Proc.new{}
    my_prc = MyProc.new(&prc)
    # => returns an instance of MyProc duplicated from prc
    MyProc.new(&my_prc)
    # => SHOULD return my_prc, but DOESN'T, rather DOES duplicate my_prc
    Proc.new(&prc)
    # => returns prc, by comparison

The problem is that there is unintended code path in which the needless
duplication can happen.

Thus, the duplication logic is moved from Proc::from_env() into the ruby code
of Proc.new and a new primitive called proc_duplicate_as_subclass.

Also revert no longer valid changes done in the following commit, which
originally tried to fix this:

1abfbdf
Make Proc::from_env reset klass if appropriate
  • Loading branch information
ryoqun committed Feb 7, 2013
1 parent 5d4baad commit 91690df
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 21 deletions.
5 changes: 5 additions & 0 deletions kernel/common/proc.rb
Expand Up @@ -38,6 +38,11 @@ def self.new(*args)

block = __from_block__(env)

if block.class != self and block.class != Method
block = block.dup
Rubinius::Unsafe.set_class(block, self)
end

Rubinius.asm(block, args) do |b, a|
run b
run a
Expand Down
15 changes: 9 additions & 6 deletions vm/builtin/proc.cpp
Expand Up @@ -34,11 +34,6 @@ namespace rubinius {

Proc* Proc::from_env(STATE, Object* self, Object* env) {
if(Proc* p = try_as<Proc>(env)) {
if(p->klass() != self &&
p->klass() != G(proc)->get_const(state, "Method")) {
p = as<Proc>(p->duplicate(state));
p->klass(state, as<Class>(self));
}
return p;
}

Expand All @@ -48,7 +43,15 @@ namespace rubinius {
return proc;
}

return static_cast<Proc*>(Primitives::failure());
return NULL;
}

Proc* Proc::from_env_prim(STATE, Object* self, Object* env) {
if(Proc* p = from_env(state, self, env)) {
return p;
} else {
return static_cast<Proc*>(Primitives::failure());
}
}

Object* Proc::call(STATE, CallFrame* call_frame, Arguments& args) {
Expand Down
4 changes: 3 additions & 1 deletion vm/builtin/proc.hpp
Expand Up @@ -36,9 +36,11 @@ namespace rubinius {
// Rubinius.primitive? :proc_call_on_object
Object* call_on_object(STATE, CallFrame* call_frame, Executable* exec, Module* mod, Arguments& args);

// Rubinius.primitive :proc_from_env
static Proc* from_env(STATE, Object* self, Object* env);

// Rubinius.primitive :proc_from_env
static Proc* from_env_prim(STATE, Object* self, Object* env);

class Info : public TypeInfo {
public:
BASIC_TYPEINFO(TypeInfo)
Expand Down
9 changes: 2 additions & 7 deletions vm/instructions.def
Expand Up @@ -2148,13 +2148,8 @@ instruction push_proc() [ -- value ]

Object* obj = call_frame->arguments->block();
if(CBOOL(obj)) {
Proc* prc;
if(BlockEnvironment *env = try_as<BlockEnvironment>(obj)) {
prc = Proc::create(state, G(proc));
prc->block(state, env);
} else if(Proc* p = try_as<Proc>(obj)) {
prc = p;
} else {
Proc* prc = Proc::from_env(state, G(proc), obj);
if(!prc) {
Exception::internal_error(state, call_frame, "invalid block type");
RUN_EXCEPTION();
}
Expand Down
9 changes: 2 additions & 7 deletions vm/llvm/jit_util.cpp
Expand Up @@ -1494,13 +1494,8 @@ extern "C" {
Object* rbx_make_proc(STATE, CallFrame* call_frame) {
Object* obj = call_frame->scope->block();
if(CBOOL(obj)) {
Proc* prc;
if(BlockEnvironment *env = try_as<BlockEnvironment>(obj)) {
prc = Proc::create(state, G(proc));
prc->block(state, env);
} else if(Proc* p = try_as<Proc>(obj)) {
prc = p;
} else {
Proc* prc = Proc::from_env(state, G(proc), obj);
if(!prc) {
Exception::internal_error(state, call_frame, "invalid block type");
return 0;
}
Expand Down

0 comments on commit 91690df

Please sign in to comment.