Misc #21154: Document or change Module#autoload?

• Status: Feedback
• Assignee: mame (Yusuke Endoh)

[fxn (Xavier Noria)]

The documentation of Module#autoload? says

Returns filename to be loaded if name is registered as autoload in the namespace of mod or one of its ancestors.

Cool, but in the following snippet

autoload :Foo, 'foo'
autoload?(:Foo)

the second line could evaluate to nil, and this does not seem to agree. I just registered an autoload, therefore (according to the documentation) I should get "foo" back in line 2.

I'd like to ask for clarification from the Ruby team:

1. Is the documentation complete? Should that second line always return "foo"?
2. If the answer is no, which is the logic missing in the docs?

Thank you!

[Eregon (Benoit Daloze)]

• Related to Misc #21035: Clarify or redefine Module#autoload? and Module#const_defined? added

[mame (Yusuke Endoh)]

I would like you to learn how to write a bug report.

the second line could evaluate to nil.

Please write a self-contained reproducible example that actually does that, not just say “could”.
Then write down the results you expect to get with that code, and the actual results.

Also, I want you to write the background (what you want to do finally), and why the current behavior is troubling for your purpose.

[mame (Yusuke Endoh)]

Should that second line always return "foo"?

No, not necessarily. The document says "if name is registered as autoload".
And the document of Kernel#autoload says "If const is defined but not as autoload, does nothing."
Thus, there is no guarantee that your second line will always return "foo".

I assume you have https://bugs.ruby-lang.org/issues/21035#note-3 in mind.
IMO, in your case, the situation could be considered equivalent to the “If const is defined but not as autoload, does nothing.” case.
Therefore, I don't think it is surprising that autoload? returns nil.

So should the documentation be updated? I personally think that is not necessary either. Documentation is documentation, not a spec. I think it is more useful to have a simple description of typical behavior and usage. Currently, I feel the document is good enough. I admit people may have different opinions here, though.

I think what you should do is not arguing about consistency with the documentation, but rather briefly explaining your purpose and why the current behavior is troubling against it.

[fxn (Xavier Noria)]

Hi @mame (Yusuke Endoh), thank for your reply.

No, not necessarily. The document says "if name is registered as autoload".
And the document of Kernel#autoload says "If const is defined but not as autoload, does nothing."
Thus, there is no guarantee that your second line will always return "foo".

That is right. However, if we have a file foo.rb

autoload :Foo, 'foo'
p autoload?(:Foo)

That prints nil if you ruby -I. -e 'require "foo"'.

So, the constant is not defined, the autoload is registered, and you get nil. This does not agree with the documentation, do you agree? You may say "circularity!". And I'll say, cool, but undocumented.

I think what you should do is not arguing about consistency with the documentation, but rather briefly explaining your purpose and why the current behavior is troubling against it.

I am not arguing, I believe my tickets cannot be more respectful, formulated as questions, and leaving the door open to an explanation of how things work that is not documented.

You do not always come to ask for questions with a use case in mind. I do use autoload? and this edge case affects me, yes. But what I am trying to accomplish is to know Ruby better.

I understand you don't want to document this (we disagree, but that is your call), but you also have to understand that as a Ruby programmer I strive to understand how Ruby works per se, that is my goal. And not being a committer, I cannot by myself know if the mismatches with the docs are intentional, an overlook, or if intentional, which is the mental model that fits that mismatch.

[fxn (Xavier Noria)]

You have also to take into account that in that example const_added is called, Module#constants lists :Foo, which is consistent with the autoload but not with autoload?.

So, the whole thing is kind of inconsistent right now.

But as a Ruby programmer, I cannot make sense of it without your help. Maybe the response of the Ruby team is "oh, that is right, `autoload? should be a fast lookup to the constants table and nothing else!", or "actually not, the way to make all this fit together is to revise such and such APIs".

[fxn (Xavier Noria)]

Finally, this ticket was intentionally not categorized as "Bug", but it is "Misc". Because I do not know if there is a bug.

[fxn (Xavier Noria)]

@mame (Yusuke Endoh) Let me put it all together:

Consider a script foo.rb:

autoload :Foo, 'foo' # could be a dynamic call
p autoload?(:Foo)    # could be a dynamic call

If we execute that with ruby -I. -rfoo -e1, we get nil. But this does not agree with the docs:

1. The constant Foo does not exist, therefore (according to the docs), the autoload is registered.
2. Module#constants lists :Foo, which is consistent.
3. Module#const_added is called with :Foo, which is consistent.

Now, the docs of Module#autoload? say:

Returns filename to be loaded if name is registered as autoload in the namespace of mod or one of its ancestors.

since the autoload is registered, I should get a filename back, not nil.

My question to the Ruby team is: Is the behavior OK and the docs should be updated, or should the implementation be updated instead?

My suggestion would be to keep the docs as they are and simplify autoload? to be a simple lookup in the constants table.

(Hope that explains it better.)

[mame (Yusuke Endoh)]

I understood as follows: You want @matz (Yukihiro Matsumoto) to clarify the behavior around autoload under circular require patterns, but you never want to explain your motivation example.

I said in #21035 that at the last dev meeting, the decision was made not to discuss anything by using code examples in the circular require pattern.
You are requesting that we discuss the same agenda item again with little new material. Frankly, such a request would only waste the time of the dev meeting and is not productive.

I don't understand why you are so concerned about circular require patterns. This is just my imagination, but you, the author of the zeitwerk, cannot avoid the presence of circular require patterns in the user code?
If so, for example, does it help you by issuing a warning or error for autoloading that would result in a circular require pattern?

[fxn (Xavier Noria)]

@mame (Yusuke Endoh) I am showing a registered autoload whose autoload? does not return a file name. If proves the documentation is wrong, or the implementation is wrong. But I do not know which is which. That is the point I am trying to make.

[fxn (Xavier Noria)]

It may also help to know that I am writing a book about Ruby constants, so it would be helpful to know if the documentation is wrong, or the implementation is wrong.

[fxn (Xavier Noria)]

@mame (Yusuke Endoh) I don't want anyone to waste their time, if you believe discussing this is so, please feel free to close.

I opened this ticket because the discussion went on in the previous one and @Eregon (Benoit Daloze), who is a Ruby committer, suggested to do so.

Why I don't show source code? Because this is not a ticket motivated by a need I have. Zeitwerk has been living with this edge case for 6 years. I am fine.

I thought that pointing out that the API is not consistent would be of your interest per se. Resolutions could be several:

• Oh, thank you, autoload? is OK, we could document it checks features.
• Oh, thank you, autoload should be the one checking features, actually, the piece that is not right here is autoload.
• Oh, thank you, autolad is fine, but autoload? should return the file name, let's fix that.

But we do not seem to be aligned. It is OK, please, feel free to close.

[mame (Yusuke Endoh)]

Why I don't show source code? Because this is not a ticket motivated by a need I have.

I see, you really do not have a use case. I finally understand.
I had misunderstood that you were hiding your use cases and somewhat challenging us.
(I am not sure if my English wording is appropriate here, sorry if it makes a wrong sense.)
Sorry for the frustration.

Let me explain why I am eager to hear the use cases.

In principle, all proposed changes to Ruby are considered based on the use cases that would require the change.
Even if it is just an inconsistency, we force ourselves to assume use cases that could be affected by the inconsistency and then consider how (and whether) to fix it.

Therefore, ticket discussions at the dev meetings begin with an understanding of the use case.
Without a background explanation in the ticket, a great deal of time is spent to try identifying use cases.
Still, it is impossible to cover all the use cases in a limited amount of time, so we could reach incomplete conclusions.
We sometimes end up finding a different use case than the one the proposer has in mind, leading to a conclusion that the users, including the proposer, do not want.
If a reasonable use case cannot be found, we need to ask the proposer to clarify, which takes time.
Or we may choose WONTFIX, or make a random choice based on matz's intuition, which is also often not the result desired by the proposer.

You may think that Ruby committers has the "answer" to the question you have and you want to just get it out of us.
In fact, we have no "answer". At least I do not have. I believe matz does not have it either for your question yet.
We need to decide how (and whether) to fix it by using the use cases.
If there is no particular use case that is troubling you, then we tend to choose "don't touch it" or just rely on matz's intuition.

As for autoload, there is a long history of subtle and repeated fine-tuning based on problems encountered in real-workd use cases.
Therefore, I feel strong resistance to changing something based on intuition alone.
I am afraid that by making a bad change, Zeitwerk stops working, which will affect many people.

This is the reason why I insisted on asking you, the author of Zeitwerk, for background on this issue.

[fxn (Xavier Noria)]

Oohhh, now I understand! Thanks for explaining this.

I am also not familiar with how dev meetings are run, so we were coming from different angles to this and I guess we were not quite understanding each other in the threads.

Let me think if it is worth explaining how usage of autoload? in Zeitwerk is affected by this (the gem has a workaround to deal with this behavior).

Let me think, I'll write back, thanks again for your clarifications @mame (Yusuke Endoh).

[fxn (Xavier Noria)]

Hi @mame (Yusuke Endoh).

My main motivation for starting this discussion was to 1) understand Ruby better, 2) bring some inconsistencies to the attention of the Ruby team and ask for clarification (here, I assumed the team knows autoload inside out and matz would instantly know the correct mental model/logic, your reply yesterday helped me understand this assumption is not entirely correct) 3) know what to write in my book.

Now that I understand better how dev meetings work and how issues are evaluated, I am not sure the use case in Zeitwerk affected by this behavior is worth a debate.

Lastly, let me address this sentence:

I had misunderstood that you were hiding your use cases and somewhat challenging us.

You can be 100% confident I'd never do that.

My questions & discussions are in good faith and with an OSS collaboration spirit.

I have a huge respect for everybody in the Ruby team. When I wrote Zeitwerk, I told Matz that if he wanted to keep the deprecation of autoload I'd cancel the project, and I meant it. Suggested to Matz in Sarajevo last year that if he wanted to remove autoload from Ruby and think of an alternative, I'd be OK and willing to help.

We may agree or disagree in specific technical details, that is normal, but always with good intention!

I think we can close, thanks @mame (Yusuke Endoh)!

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Closed

[mame (Yusuke Endoh)]

• Status changed from Closed to Assigned
• Assignee set to mame (Yusuke Endoh)

This was discussed at the dev meeting.

It is decided to add a sentence to documents of Kernel#autoload and Module#autoload that prohibits registering a file being loaded. I have created a PR.

https://github.com/ruby/ruby/pull/12926

It was also discussed to determine this condition at runtime and raise a warning or exception. However, the overhead of resolving file path when autoload is called may be a problem for programs that set a large number of autoloads, so it is not decided.

[fxn (Xavier Noria)]

Thanks @mame (Yusuke Endoh)!

I believe this may not entirely work.

That makes sense to be asked if you are thinking about explicit source code doing the autoload.

However, in situations in which the autoloads are set dynamically, as it is the case with Zeitwerk, you don't have API to know if a given file is being loaded at the moment.

[mame (Yusuke Endoh)]

• Status changed from Assigned to Closed

Applied in changeset git|ee1f39ef882e7ce175794e6286c0dcafba0bfa35.

---

Add a document to autoload

Users are responsible for avoiding circular autoload.

[Misc #21154]

[mame (Yusuke Endoh)]

• Status changed from Closed to Feedback

@fxn I think it makes sense. Would it help you if such a new API is introduced to determine if a file is currently being loaded?

[fxn (Xavier Noria)]

@mame (Yusuke Endoh) thanks for considering.

I am not sure it would be a good idea in practice for the same reason it was decided to not do that when Module#autoload is invoked, it would be too costly.

Zeitwerk needs to keep track of these edge cases, I call them "inceptions" in the source code (usage example). Checking that would be prohibitive, because in a project managed by Zeitwerk everything has an autoload call set. Even when eager loading (for consistency, eager loading is a recursive const_get, not a recursive require).

If I ever come with a concrete alternative, I may followup, but meanwhile perhaps we can leave it here.

[fxn (Xavier Noria)]

Hi @mame (Yusuke Endoh).

After thinking about this, I have an important use case that I'd like to share, because it conflicts with that documentation patch.

I recorded this video to explain it to you and all the team.

[fxn (Xavier Noria)]

Hi @mame (Yusuke Endoh), did you have a chance to watch the video or discuss it in a dev meeting?

[mame (Yusuke Endoh)]

@fxn Sorry for the delay. I finally watched your video.

I'm not exactly sure what you meant by "const_get hook", but perhaps you meant "const_added"? I'll assume that's the case and continue.

Here's how I understand the issue.

Suppose we have a gem with lib/foo.rb and lib/foo/bar.rb, and an app.rb that requires it:

# lib/foo.rb
Zeitwerk::Loader.for_gem.setup

module Foo
end

# lib/foo/bar.rb
module Foo
  class Bar
  end
end

# app.rb
require "foo"

Foo::Bar

In this setup, Zeitwerk::Loader.for_gem.setup in lib/foo.rb effectively does something like this:

# Since lib/foo.rb exists, it sets autoload for ::Foo
autoload :Foo, "lib/foo"

# Then, using the const_added hook, it sets autoload for Foo::Bar when Foo is defined:
class Object
  def self.const_added(name)
    if name == :Foo
      Foo.autoload(:Bar, "lib/foo/bar")
    end
  end
end

This means that lib/foo.rb sets autoload :Foo, "lib/foo", which would violate the rule of my documentation patch.

Is this understanding correct?

---

If that's the case, I think lib/foo.rb is doing something quite tricky. Let me show a simpler example:

# test.rb
p 1
autoload :Foo, "./test"
p 2
class Foo
  p 3
end

Running this code produces the following output:

$ ruby test.rb
1
2
1
2
3
3

This shows that class Foo triggers the autoload and ends up circularly loading test.rb itself. As a result, the file ends up being evaluated twice.

Is this kind of circular loading what actually happens in Zeitwerk::Loader.for_gem.setup? If so, is this really the intended behavior?

---

By the way, I appreciate your kindness in recording a video, but to be honest, I'm not very good at understanding spoken English. I'd appreciate it if you could explain things in text and using code examples.

Also, this month's dev meeting will be held at RubyKaigi, a Japanese conference, and will be shorter than usual, so I don't think we'll have time to discuss this topic. I'm sorry, but I think we'll need to revisit it in the May dev meeting.

[mame (Yusuke Endoh)]

Aha, when test.rb is required, currently autoload does (almost) nothing because test.rb is already being required, so test.rb is never executed twice.

$ ruby -r./test.rb -e ''
1
2
3

Also, when I execute action_cable.rb directly, I got constant redefinition warnings. This means action_cable.rb was indeed evaluated twice :-)

$ ruby /home/mame/.rbenv/versions/ruby-dev/lib/ruby/gems/3.5.0+0/gems/actioncable-8.0.2/lib/action_cable.rb
/home/mame/.rbenv/versions/ruby-dev/lib/ruby/gems/3.5.0+0/gems/actioncable-8.0.2/lib/action_cable.rb:58: warning: already initialized constant ActionCable::INTERNAL
/home/mame/.rbenv/versions/ruby-dev/lib/ruby/gems/3.5.0+0/gems/actioncable-8.0.2/lib/action_cable.rb:58: warning: previous definition of INTERNAL was here

I think I understand the problem and can explain it at the May dev meeting. However, I am not sure what is the best solution for this issue.

[fxn (Xavier Noria)]

Hi @mame (Yusuke Endoh)!

I'm not exactly sure what you meant by "const_get hook", but perhaps you meant "const_added"? I'll assume that's the case and continue.

Right, I had a lapsus there.

This means that lib/foo.rb sets autoload :Foo, "lib/foo", which would violate the rule of my documentation patch.

Exactly, I could not comply with the new documentation without breaking hundreds of gems. And there are more existing edge cases, I only showed the most important one.

Since I know compatibility is something the Ruby team has as a high priority, I believed I had to raise this concern.

And, besides these examples, code issuing dynamic autoloads cannot comply with the docs either, because it has no way to check if a given file is being loaded at the moment. But this point is kind of more theoretical, while the point above is very real.

By the way, I appreciate your kindness in recording a video, but to be honest, I'm not very good at understanding spoken English. I'd appreciate it if you could explain things in text and using code examples.

My pleasure. I'll remember that for other tickets.

Regarding your remarks about loading the entrypoint by passing it to the interpreter, or to Kernel#load, etc. these are documented to not be supported:

"Due to technical reasons, the entry point of the gem has to be loaded with Kernel#require, which is the standard way to load a gem. Loading that file with Kernel#load or Kernel#require_relative won't generally work."

I think I understand the problem and can explain it at the May dev meeting. However, I am not sure what is the best solution for this issue.

Excellent, thank you.

The discussion here was related to the predicate Module#autoload?, I guess you guys ended up addressing this one by disallowing circularity altogether, but (as you mentioned in some comment), that turns to uncover conflicting stuff that perhaps was not foreseen during the discussion.

May I suggest restoring the original documentation?

Thanks again, hope you folks have a great time in RubyKaigi.

[mame (Yusuke Endoh)]

I had a quick chat with @akr (Akira Tanaka).

Zeitwerk::Loader.for_gem.setup identifies its caller file (i.e., the entrypoint file of the gem) by using caller_locations.

https://github.com/fxn/zeitwerk/blob/67b1396cab1bb517b9a63833417831d648794fcc/lib/zeitwerk/loader.rb#L391

Because the entrypoint file is already requireed, Zeitwerk doesn't have to set autoload on the file.
So, it is possible to avoid the circular autoload, and follow the new limitation introduced by my documentation change, right?

[fxn (Xavier Noria)]

Perhaps that would work for this exact use case.

But it would be ad-hoc and limited, it would not be addressing the conflict at its root cause. As documented, for_gem is only a convenience shortcut for a common use case. Gems can use the generic API, instantiate the loader in arbitrary places, and some do.

And I have more use cases, like a gem that migrates to Zeitwerk and existing client code has require calls for files inside the gem. Nowadays, those calls work.

All of this works today, it even has test coverage for other interpreters.

If the change was: "If the autoload is circular, it is ignored and the return value says so", I believe I could modify the gem to take that into account, provided that nowadays I actually have that logic by asking autoload? after it. But it was discarded in a previous comment as "too costly" (from memory).

But, the key point is you cannot honor what the docs say in master. I suspect they have static usage in mind, but are not considering dynamic usage where, as far as I know, you do not have the tools to check if an arbitrary file is being loaded at the moment, so you cannot know if you can set an autoload, and strictly speaking then you cannot set ANY autoload.

Let's do this experiment:

1. I'd like to set an autoload for this file dynamically.
2. As per the docs, you cannot do that if the file is being loaded.
3. How can I check that?
4. You cannot (unless I am missing some API).
5. Therefore, I cannot set the autoload and comply with the docs.