Improvements to ActiveSupport::Dependencies missing constant loading

rubyonrails-core
1

Hi,

I have a specific case where the rails constant loading algorithm seems to be not optimal. Here is an example:

Suppose there are constants A::M and A::b::M, defined in correctly-named files in the autoload path. Assume A::b::M has not yet been loaded. When the following is loaded:

module A::B class C M end end

``

There are two possible behaviours, based on whether or not A::M has been loaded. This seems unexpected and is frequently undesirable to me. I would expect A::b::M to be loaded by missing constant loading algorithm.

So, we have two cases:

  1. A::M is not already loaded

Rails tries to load, in order, A::b::C::M, A::b::M, A::M, ::M

  1. A::M is already loaded

Rails tries to load only A::b::C::M. The missing constant loading algorithm will attempt to load A::b::C::M, which fails. However, because A::M exists, it will not attempt to load A::b::M, and a name error is raised.

The (vanilla) ruby order for these constants would be: A::b::C::M, A::b::M, ::M

It seems to me that case 2) is not optimal, and not hard to fix. Because A::M is defined but was not loaded by ruby (that is, A was not part of Module.nesting and so const_missing was invoked), we know that A::M is not what would be referenced according to the ruby constant rules. However, it seems that it would be preferable to attempt to load A::b::M, to be more in line with case 1. It also seems reasonable to attempt to load ::M.

These are things that the const loading algorithm seems as though it could do, and I am willing to make a patch for that. Is there something I’m missing here, or some reason it does not attempt to load these constants?

Arron

2

In general, AS::Dependencies has to be seen in a positive way: AS::Dependencies performs a particular file lookup given a constant missing in a certain class or module. If I make it, that contract will be documented in a new guide for 4.2 I am working on.

In particular, AS::Dependencies cannot be seen as an attempt to emulate Ruby loading algorithms. It is based on const_missing, which does not get passed two key metadata:

1) The nesting.

2) Whether the missed constant was relative or qualified.

Without that information algorithms cannot be emulated and AS::Dependencies in consequence does not even pretend to emulate them.

In that sense, my personal opinion is that the special rule you mentioned that checks A::M to try to guess relative/absolute is so misleading it should not be there. As your example shows it yields unexpected behavior (if one expects Ruby rules), and it depends on load order. If by luck that was already loaded, we act some way, if not yet loaded, in a different way. That's not good IMO and if it weren't for backwards compatibility I would be inclined to remove it.

Given all this context, I don't think it would be a good idea to load the constant your proposal suggests to load. On which grounds would AS::Dependencies special-case that the rightmost segment would have a special treatment if it lacks the nesting? And why

should attempt to load A::b::M? That would be really surprising and AS::Dependencies is not able to distinguish between your example and the constant path above. All it knows is a constant called "M" has been missing in a lookup where a class called "A::b::C" is the first element of the nesting. That's all you know.

It's all very confusing if you see it from a Ruby angle, that's why I stress you need to forget the Ruby point of view and just program against the AS::Dependencies contract (unfortunately undocumented, but working on it).

In general my recommendation is to use relative paths as much as possible, specially after the class and module keywords, and make judicious use of require_dependency for edge cases like this.

Xavier

PS: If constant autoloading could be implemented with Kernel#autoload everything would be consistent, but unfortunately that's impossible due to a number of reasons.

3

should attempt to load A::b::M? That would be really surprising and

AS::Dependencies is not able to distinguish between your example and the constant path above. All it knows is a constant called "M" has been missing in a lookup where a class called "A::b::C" is the first element of the nesting. That's all you know.

Rather: All it knows is a constant called "M" has been missing from class called "A::b::C", it does not even know if it was the first element in the nesting (counterexample in A::b::C::M).

4

Hi Xavier,

Thanks for the reply.

5

Consider:

Case A)

  module A     module B       class C         M       end     end   end

Case B)

  module A::B     class C       M     end   end

Case C)

  class A::b::C     M   end

Case D)

  class X::Y::Z     class A::b::C       M     end   end

Case E)

We have 5 examples. In every one of them Ruby resolves in a different way, but AS::Dependencies cannot because in all of them it only knows that a constant "M" was missing in a class whose name is "A::b::C", that's all.

In case D, for example, A::b::M should *not* be checked, but X::Y::Z::M should be checked. The proposal fixes one case, but breaks some others equally valid in addition to constant paths (whose detection is the motivation for the special rule we both agree is a dubious idea).

There's no solution, in general you cannot design AS::Dependencies with emulation in mind. It does not even lookup in "ancestor" paths!

It doesn't emulate, it cannot emulate, it has its own rules.

6

There’s no solution, in general you cannot design AS::Dependencies with emulation in mind. It does not even lookup in “ancestor” paths!

It doesn’t emulate, it cannot emulate, it has its own rules.

I’m convinced that emulation of the ruby behaviour is not possible. However, I’m still not fully understanding this. My example is really about how the behaviour differs based on whether or not certain constants are already loaded – all I want to do is handle the “constant already loaded” case in the same way as the “constant not yet loaded” case. As far as I can tell, I’m not really proposing a new behaviour, because it is already the behaviour one gets if the constant has not yet been loaded. I’m still trying to understand if there’s a reason why it cannot work like this, or if it is just for backwards compatibility. Your original reply somewhat addressed this:

7

There's no solution, in general you cannot design AS::Dependencies with emulation in mind. It does not even lookup in "ancestor" paths!

It doesn't emulate, it cannot emulate, it has its own rules.

I'm convinced that emulation of the ruby behaviour is not possible. However, I'm still not fully understanding this. My example is really about how the behaviour differs based on whether or not certain constants are already loaded -- all I want to do is handle the "constant already loaded" case in the same way as the "constant not yet loaded" case. As far as I can tell, I'm not really proposing a new behaviour, because it is already the behaviour one gets if the constant has not yet been loaded. I'm still trying to understand if there's a reason why it cannot work like this, or if it is just for backwards compatibility.

The proposal is, couldn't we try A::b::M in

    module A::B       class C         M       end     end

even if A::M existed?

Question, why?

Case 1) Because Ruby would.

Answer 1) Ruby wouldn't in

    class A::b::C       M     end

and const_missing has no way to know in which one of the cases the call happened. So that would fulfill the expectation in one case to break it in another case. Emulating Ruby is not a design driver for AS::Dependencies because it cannot be done.

Case 2) To have it behave the same regardless of the existence of A::M.

Answer 2) We agree this rule is dubious, but it is the rule that prevents AS::Dependencies from walking parent namespaces in

and we cannot change that rule at this point in the game due to backwards compatibility.

8

In general, AS::Dependencies has to be seen in a positive way:

AS::Dependencies performs a particular file lookup given a constant missing in a certain class or module. If I make it, that contract will be documented in a new guide for 4.2 I am working on.

Yep, the guide is going to be out with 4.2:

There is a section about gotchas, this particular subsection is relevant to this thread: