Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #10470 (closed defect: fixed)

Opened 7 months ago

Last modified 7 months ago

[PATCH] Class#const_missing should bypass Dependencies.load_missing_constant where possible (Ruby 1.9 compat)

Reported by: chuyeow Assigned to: core
Priority: normal Milestone: 2.x
Component: ActiveSupport Version: edge
Severity: normal Keywords: ruby 1.9 yarv
Cc:

Description

Mostly a Ruby 1.9 compatibility patch - in 1.9, Module#const_defined? accepts a 2nd parameter, a flag specifying whether ancestors will be included. E.g.

>> RUBY_VERSION
=> "1.8.6"
>> module Parent; FOO = 'bar'; end
>> module Child; include Parent; end
>> Child.const_defined?('FOO')
=> false
>> RUBY_VERSION
=> "1.9.0"
>> module Parent; FOO = 'bar'; end
>> module Child; include Parent; end
>> Child.const_defined?('FOO')
=> true

Rails redefines Module#const_missing so that it can autoload associations, but as a result Class#const_missing is also redefined (Class < Module anyway). I think the whole dependencies.rb is in need of a refactor anyway, but at least this patch assures Ruby 1.9 compatibility.

Oh, this fixes test failures in 1.9 like:

  1) Error:
test_inspect(DurationTest):
ArgumentError: ActiveSupport is not missing constant Hash!
    /foo/bar/rails_trunk/activesupport/lib/active_support/dependencies.rb:240:in `raise'
    /foo/bar/rails_trunk/activesupport/lib/active_support/dependencies.rb:240:in `load_missing_constant'
    /foo/bar/rails_trunk/activesupport/lib/active_support/dependencies.rb:453:in `const_missing'
    /foo/bar/rails_trunk/activesupport/lib/active_support/dependencies.rb:476:in `rescue in const_missing'
    /foo/bar/rails_trunk/activesupport/lib/active_support/dependencies.rb:473:in `const_missing'
    /foo/bar/rails_trunk/activesupport/lib/active_support/duration.rb:57:in `inspect'
    ./test/core_ext/duration_test.rb:5:in `test_inspect'

Attachments

const_missing_ruby_1.9_compat.diff (2.3 kB) - added by chuyeow on 12/11/07 15:34:22.
basic_object_1.9_resolving.diff (0.6 kB) - added by chuyeow on 12/12/07 12:07:04.

Change History

12/11/07 15:34:22 changed by chuyeow

  • attachment const_missing_ruby_1.9_compat.diff added.

12/11/07 16:36:01 changed by fxn

I agree with the parameter renaming, and with the feeling that dependencies.rb needs a refactoring.

I don't understand that new line though. Would you please give an example where ::Object.const_defined?(const_name) is true but Class#const_missing is anyway invoked? See for example

$ cat lib/foo/bar.rb                                 
module Foo
  class Bar; end
end
fxn@feynman:~/aspgems/prj/facturagem/trunk$ cat lib/bar.rb                                     
class Bar; end
fxn@feynman:~/aspgems/prj/facturagem/trunk$ script/runner 'module Foo; puts Bar.name; end'     
Foo::Bar
fxn@feynman:~/aspgems/prj/facturagem/trunk$ script/runner 'Bar; module Foo; puts Bar.name; end'
Bar

12/11/07 16:51:32 changed by chuyeow

Sure :)

I'm getting the test failure described above in duration_test.rb.

duration_test.rb uses the Duration class, which defines an #inspect method that uses Hash.new. In this case, Class#const_missing will be invoked, and without intervention, will invoke the redefined Class#const_missing in dependencies.rb.

That's all fine and good in Ruby 1.8, since const_missing doesn't look up the ancestors for the constant by default. The current implementation of const_missing and Dependencies.load_missing_constant takes care of everything. But with Ruby 1.9 const_missing looks up the ancestors by default, and that messes things up (hence the stack trace).

My patch works around that by trying to fetch the constant from Object first, bypassing the entire constant lookup process if possible. Surely a cleaner refactoring of the const_missing methods to accommodate the changed behavior in Ruby 1.9 is more elegant, but this is a faster win imo until someone looks into refactoring dependency handling properly.

12/11/07 16:53:57 changed by chuyeow

I have to add, all this could be only happening on my setup if I've not set up Ruby 1.9 correctly, so please help verify that the test is failing for you too!

12/11/07 23:52:23 changed by josh

All tests pass under Ruby 1.86.

This patch fixes "test_inspect(DurationTest)" under Ruby 1.9.

12/12/07 11:15:01 changed by fxn

I can't understand why const_missing is triggered in that test for constant Hash:

fxn@feynman:~/tmp$ cat test_const_missing.rb                 
class Class
  def const_missing(const_name)
    puts "const missing called for #{const_name}"
  end
end

class Foo
  def inspect
    Hash.new
  end
end

puts Foo.new.inspect
fxn@feynman:~/tmp$ ruby -v test_const_missing.rb
ruby 1.8.6 (2007-09-23 patchlevel 110) [i686-darwin8.10.1]

fxn@feynman:~/tmp$ $HOME/ruby19/bin/ruby -v test_const_missing.rb
ruby 1.9.0 (2007-12-12 patchlevel 0) [i686-darwin9.1.0]
{}

But the fact is that it is triggered and the test fails in 1.9. Do you understand why? I mean, the attempt to resolve the constant is done by the interpreter _before_ const_missing is invoked. If the constant is in scope as it happens with Hash the redefinition of the semantics of that method would not break that test.

12/12/07 11:39:11 changed by chuyeow

I totally understand you point since that is what I expected the interpreter to do (which is to resolve the constant before const_missing is invoked).

However it seems that subclassing BasicObject changes things. Try this out in an irb session in 1.9:

class Module
  def const_missing(const)
    puts "Module#const_missing for #{const}"
  end
end

class Class
  def const_missing(const)
    puts "Class#const_missing for #{const}"
  end
end

module A
  class Foo < BasicObject
    def initialize
    end
    def inspect
      Hash.new
    end
  end
end

A::Foo.new

You should see Class#const_missing getting invoked (before a error on #new - not sure of BasicObject semantics at this point to know why). I suspect this is because BasicObject is an ancestor of Object, which is why Hash is not in scope at that point:

Object.ancestors
=> [Object, Kernel, BasicObject]

I don't know if this is expected behavior or a Ruby 1.9-dev bug though. I'll look around to see if I can find any info.

12/12/07 12:03:06 changed by chuyeow

I think I understand the problem now.

Compare:

class Foo < BasicObject
  def initialize
  end
  def inspect
    Hash.new
  end
end

Foo.new

to

class Foo < BasicObject
  def initialize
  end
  def inspect
    ::Hash.new
  end
end

Foo.new

Explicitly setting the lookup for Hash from the root namespace (not sure if this is the right terminology) works (as expected).

Just to be sure it wasn't some Ruby scoping (when subclassing) that I wasn't understanding correctly:

class Foo; end
class Bar < Foo
  def inspect
    Hash.new
  end
end

Bar.new

Works fine. So looks like since BasicObject is a top-level class and doesn't have those Object methods we're so fortunate to have, is incapable of resolving constants.

At least that is how I understand it now, but please correct me if I'm wrong!

Will attach a new patch.

12/12/07 12:07:04 changed by chuyeow

  • attachment basic_object_1.9_resolving.diff added.

12/12/07 12:08:47 changed by chuyeow

How stupid the patch looks now! At least I learnt something about BasicObject and BlankSlate.

12/13/07 00:43:08 changed by fxn

Good :-).

Problem is BasicObject is a new class in 1.9 that has almost nothing and is the parent of Object. On the other hand top-level constants are stored in Object in 1.9 (as well as in 1.8).

Duration is a direct child of BasicObject and thus does not have Object among it's ancestors. Therefore a bare "Hash" is not found according to constant name resolution in 1.9. The syntax ::Hash means "look for the constant Hash in the top-level", which is Object. Right, Object::Hash is there. As long as Duration is written that way that second patch is needed for the test to be correct in both versions of Ruby.

Nevertheless I guess dependencies.rb will need a revision to reflect Ruby 1.9 rules.

12/15/07 02:31:51 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.