-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
defined? with a colon2 fully resolves the left constant #3903
Comments
This is even a more generic issue than even Colon{2,3}. I can see that for bare constants in test_defined.rb from MRI test suite that we are blowing up as we try to autoload a non-existent file for a constant (referenced from defined). |
As a quick followup here is MRI test which breaks with a simple constant: class TestAutoloadedNoload
autoload :A, "a"
def a?
defined?(A)
end
end
p TestAutoloadedNoload.new.a? This is broken in all versions of JRuby. I am not actually sure these are the same problem but it appears we are fully resolving autoloads in defined? so it is not implausible the const_missing logic is the same code path. |
So in fact, both lexical and inheritance search instrs which are only used by defined? now have are performing autoload resolution. Believe it or not though these instrs are not the entire problem listed originally in this bug. The bug here is that our defined? logic for colon2 will end up doing a 'build(leftNode)' which is the same as calling the constant outright. This should be buildGetDefinition(leftNode) but also with some minor changes since we need to check return value instead of catching error case it in a protected block. |
Follow up message to document the behavior a bit more. My previous message is not really right. defined? will autoload the lhs of a colon2 but not the rhs. So if I have: a.rb LOADED_ME=1
module FOO
module A
B = 1
end
end and defined2.rb: module FOO
autoload :A, 'a'
end
p defined?(FOO::A)
module FOO
autoload :A, 'a'
end
module FOO
autoload :A, 'a'
end
p defined?(FOO::A)
p defined?(LOADED_ME)
p defined?(FOO::A::B)
p defined?(LOADED_ME) The first defined will have A as lhs and it will not autoload for LOADED_ME will not be defined, but once we add 'B' then A is lhs of a colon2 for B. Then we can see the autoload fire. |
@enebo Isn't this fixed with your defined? const2 rework? |
@headius weirdly I still have a failing test case on this one. So some of it was fixed but it needs some more work. |
I did solve part of this but I do not want to chance fixing the rest right before release. |
This appears to have been fixed as of at least 9.1.15. @enebo You did some work on nested constants, perhaps that was it? |
We are not compiling
defined?(UndefinedConst::XYZ)
properly.Environment
JRuby 9.1, and probably all 9k.
Expected Behavior
In 1b6f75d, @eregon added an expectation that the above syntax would not trigger a call to Object.const_missing, as in MRI. Both the XYZ and the UndefinedConst are to be looked up "soft" without errors, autoload triggering, or calls to const_missing.
Actual Behavior
We appear to be calling const_missing for the LHS of a colon2, and compiling it as a normal constant lookup rather than a "soft"
defined?
lookup.This will need to be fixed.
@enebo @subbuss This could be the source of occasional constant lookup oddities and bugs we get reported, since we're trying to resolve things that aren't meant to be resolved (or earlier than intended to be resolved).
The text was updated successfully, but these errors were encountered: