Project

General

Profile

Actions

Feature #11547

closed

remove top-level constant lookup

Added by gucki1 (Corin Langosch) over 8 years ago. Updated about 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:<unknown>]

Description

If ruby cannot find a class in the specified scope it uses the top-level constant of the same name if it exists and emits a warning:

irb(main):006:0> class Auth; end
=> nil
irb(main):007:0> class Twitter; end
=> nil
irb(main):008:0> Twitter::Auth
(irb):8: warning: toplevel constant Auth referenced by Twitter::Auth
=> Auth

In some cases this is not playing nicely with rails autoloading as can be seen here: https://github.com/rails/rails/issues/6931. Many more issues like this exist.

Imo I don't see any reason why this fallback makes any sense. So I'd like to suggest to remove it completely or at least add an option to disable it.


Related issues 3 (1 open2 closed)

Related to Ruby master - Bug #14148: Longstanding behavior regarding correspondence of toplevel with Object is surprisingClosedActions
Related to Ruby master - Bug #14407: defined? still returning true for top-level constant when referenced with scopeClosedActions
Related to Ruby master - Bug #18622: const_get still looks in Object, while lexical constant lookup no longer doesOpenActions

Updated by shugo (Shugo Maeda) over 8 years ago

  • Status changed from Open to Feedback

Corin Langosch wrote:

If ruby cannot find a class in the specified scope it uses the top-level constant of the same name if it exists and emits a warning:
(snip)
Imo I don't see any reason why this fallback makes any sense. So I'd like to suggest to remove it completely or at least add an option to disable it.

How should the following code behave?

class Foo
  X = 1
end

module Bar
  Y = 2
end

class Baz < Foo
  include Bar
end

p Baz::X
p Baz::Y

Updated by gucki1 (Corin Langosch) over 8 years ago

Hi Shugo. Just as it does now, it doesn't perform any magic (fallback to some other scope) and doesn't emit any warning. How is it related to my bug report/ feature request? Cheers, Corin.

Updated by shugo (Shugo Maeda) over 8 years ago

Corin Langosch wrote:

Hi Shugo. Just as it does now, it doesn't perform any magic (fallback to some other scope) and doesn't emit any warning. How is it related to my bug report/ feature request? Cheers, Corin.

In my example, Baz::X and Baz::Y refer to constants defined in ancestors of Baz.
In your example, Twitter::Auth also refers to a constant defined one of its ancestors, Object.

So, I'd like to clarify what behavior do you want in these cases.

Instead of changing the behavior of constant lookup, we may be able to introduce a variant of const_missing which is invoked when a constant is not directly defined in the target class.

Updated by bronson (Scott Bronson) about 8 years ago

Corin, I completely agree. Recently, Rails's nondeterministic autoload made it very hard for me to discover this problem. Without the insightful warning, I would have been sunk.

Shugo Maeda wrote:

Instead of changing the behavior of constant lookup, we may be able to introduce a variant of const_missing which is invoked when a constant is not directly defined in the target class.

Couldn't Ruby 3.0 just raise an error instead? Shugo, your example seems to demonstrate that Ruby is smart enough to realize when the behavior is intentional vs. when it's probably an accident.

I'd be happy to bang together a patch if the concept seems sound.

Updated by shugo (Shugo Maeda) about 8 years ago

Scott Bronson wrote:

Corin, I completely agree. Recently, Rails's nondeterministic autoload made it very hard for me to discover this problem. Without the insightful warning, I would have been sunk.

Shugo Maeda wrote:

Instead of changing the behavior of constant lookup, we may be able to introduce a variant of const_missing which is invoked when a constant is not directly defined in the target class.

Couldn't Ruby 3.0 just raise an error instead? Shugo, your example seems to demonstrate that Ruby is smart enough to realize when the behavior is intentional vs. when it's probably an accident.

I'd be happy to bang together a patch if the concept seems sound.

The behavior change might be acceptable in Ruby 3.0 if Matz wants.
Experiments with real world applications such as Rails might help his decision.

Updated by matz (Yukihiro Matsumoto) almost 8 years ago

I am for this proposal, but also concern about code breakage. Let's try removing top-level constant look-up in 2.4dev and see how much code it breaks.

Matz.

Updated by tisba (Sebastian Cohnen) about 7 years ago

With 2.4 released and I wasn't able to find anything related to this issue in https://github.com/ruby/ruby/blob/v2_4_0/NEWS I guess it did not make it into 2.4.

I was wondering if there are any plans regarding working on this. In almost all projects I encounter this issue and it would be great if this could improve.

Updated by nobu (Nobuyoshi Nakada) about 7 years ago

Sorry, missed this feature.

Actions #9

Updated by nobu (Nobuyoshi Nakada) about 7 years ago

  • Status changed from Feedback to Closed

Applied in changeset r57244.


variable.c: top-level constant look-up

  • variable.c (rb_const_search): [EXPERIMENTAL] remove top-level
    constant look-up. [Feature #11547]

Updated by bronson (Scott Bronson) about 7 years ago

The fix is even simpler than what I was picturing: https://github.com/ruby/ruby/commit/44a2576f798b07139adde2d279e48fdbe71a0148

If it sticks, this will make a bunch of Rails autoload issues go away. Right on nobu, thanks!

Updated by fxn (Xavier Noria) about 7 years ago

In particular this one http://guides.rubyonrails.org/autoloading_and_reloading_constants.html#when-constants-aren-t-missed.

We'd love to base Rails autoloading on Kernel#autoload and have the exact same semantics, but that is not possible as of this writing (http://guides.rubyonrails.org/autoloading_and_reloading_constants.html#module-autoload-isn-t-involved). So, Rails does not attempt to emulate constant resolution at all (it does not have nesting information, it does not follow ancestor chains by design, etc.), you have to think autoloading as a feature with its own contract. You have to program against that contract.

A change like this one is welcome though. Thanks.

Updated by zenspider (Ryan Davis) about 7 years ago

I would honestly rather see it raise an exception instead of silently return nil. I'd rather KNOW my bug than hunt it.

Updated by fxn (Xavier Noria) about 7 years ago

Oh, didn't look at the patch. This means String::Hash returns nil?

Updated by fxn (Xavier Noria) about 7 years ago

Ah, no, https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/57244/entry/variable.c#L419 seems to say that Qundef is what the search function returns to indicate failure, raising in the caller.

Updated by fxn (Xavier Noria) over 6 years ago

I realized that Object does not halt the lookup, but it is rather just skipped. For example, constants in ancestors of Object are still accessible as qualified constants:

module Kernel
  X = 1
end

X         # top-level accessible constant
String::X # still accessible this way

Is that intentional?

Updated by RickHull (Rick Hull) over 6 years ago

Hm, this is surprising:

module Kernel
  X = 1
end

puts String::X

X = 2

puts String::X
$ ruby test.rb
1
Traceback (most recent call last):
test.rb:9:in `<main>': uninitialized constant String::X (NameError)
Did you mean?  X

$ ruby --version
ruby 2.5.0preview1 (2017-10-10 trunk 60153) [x86_64-linux]
Actions #17

Updated by mrkn (Kenta Murata) over 6 years ago

  • Related to Bug #14148: Longstanding behavior regarding correspondence of toplevel with Object is surprising added
Actions #18

Updated by Eregon (Benoit Daloze) about 2 years ago

  • Related to Bug #14407: defined? still returning true for top-level constant when referenced with scope added

Updated by Eregon (Benoit Daloze) about 2 years ago

There is some inconsistency here between literal constant lookup and the meta API (const_get).
const_get still looks in Object, even though that's confusing, inconsistent and IMHO shouldn't really happen.

module ConstantSpecsTwo
  Foo = :cs_two_foo
end

module ConstantSpecs
end

p ConstantSpecs.const_get("ConstantSpecsTwo::Foo") # => :cs_two_foo
p ConstantSpecs::ConstantSpecsTwo::Foo # => const_get.rb:9:in `<main>': uninitialized constant ConstantSpecs::ConstantSpecsTwo (NameError)
Actions #20

Updated by Eregon (Benoit Daloze) about 2 years ago

  • Related to Bug #18622: const_get still looks in Object, while lexical constant lookup no longer does added

Updated by fxn (Xavier Noria) about 2 years ago

(Pervious comment moved to #18622)

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0