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

Weird behavior with #const_get and #const_defined? #4877

Closed
siutin opened this Issue Nov 30, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@siutin

siutin commented Nov 30, 2017

Environment:

uname -a
Linux 4.4.0-98-generic #121-Ubuntu SMP Tue Oct 10 14:24:03 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
ruby -v                                                                                                                                                                                                
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 OpenJDK 64-Bit Server VM 25.151-b12 on 1.8.0_151-8u151-b12-0ubuntu0.16.04.2-b12 +jit [linux-x86_64]

Below two methods work fine with ruby 2.2, 2.3 and 2.4.

#./lib/my_module.rb

# Dynamically create nested modules
def register_module(module_name)
  module_names = module_name.split("::").inject([]) { |n, c| n << (n.empty? ? [c] : [n.last] + [c]).flatten }
  module_names.each do |module_name_array|
    *prefix, parent, child = module_name_array
    if child
      full_path_child_module_name = module_name_array.join("::")
      if Object.const_defined?(full_path_child_module_name)
        puts "full_path_child_module_name: #{full_path_child_module_name} -> defined"
      else
        puts "full_path_child_module_name: #{full_path_child_module_name} -> not defined yet"
        parent_module = find_or_register_module(parent, prefix)
        parent_module.const_set child, Module.new
      end
    else
      find_or_register_module(parent, prefix) # register parent_module
    end
  end
end

def find_or_register_module(name, prefix)
  full_path_module_name = (prefix + [name]).join("::")
  if Object.const_defined? full_path_module_name
    Object.const_get full_path_module_name
  else
    if prefix.empty?
      Object.const_set name, Module.new
    else
      full_path_prefix = prefix.join("::")
      prefix_module = Object.const_get(full_path_prefix)
      prefix_module.const_set name, Module.new
    end
  end
end

spec

require 'spec_helper'
require './lib/my_module'
RSpec.describe do
  context do
    let(:module_name) { "Abc123::Foo::Bar" }
    before do
      register_module("Bar")
      register_module(module_name)
    end
    it { expect(Object.const_get(module_name)&.to_s).to match(module_name) }
  end
end

Expected Behavior

ruby 2.3.5

full_path_child_module_name: Abc123::Foo -> not defined yet
full_path_child_module_name: Abc123::Foo::Bar -> not defined yet

1 example, 0 failures, 1 passed

Actual Behavior

jruby 9.1.14.0

full_path_child_module_name: Abc123::Foo -> not defined yet
full_path_child_module_name: Abc123::Foo::Bar -> defined

expected "Bar" to match "Abc123::Foo::Bar"
./spec/jruby_bug_spec.rb:10:in `block in (root)'
-e:1:in `<main>'

1 example, 1 failure, 0 passed
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Dec 1, 2017

Member

This is likely a difference in when we determine the name for a given class. If you can reduce this it would help...the code you have here is pretty dense and I'll have to tease it apart myself to find the problem.

Member

headius commented Dec 1, 2017

This is likely a difference in when we determine the name for a given class. If you can reduce this it would help...the code you have here is pretty dense and I'll have to tease it apart myself to find the problem.

@headius headius added this to the JRuby 9.1.16.0 milestone Dec 1, 2017

@siutin

This comment has been minimized.

Show comment
Hide comment
@siutin

siutin Dec 1, 2017

@headius Thanks for the follow up. The problem is that I can reproduce this bug only if #const_defined? and #const_get are executed inside an iterator. I agree there's too many codes but still ... not so sure how to simplify this now

siutin commented Dec 1, 2017

@headius Thanks for the follow up. The problem is that I can reproduce this bug only if #const_defined? and #const_get are executed inside an iterator. I agree there's too many codes but still ... not so sure how to simplify this now

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Dec 5, 2017

Member

@siutin Provide a simple executable example without spec and try to peel off any lines of code that aren't related. I can think of no reason why running inside an iterator/block would have any effect on these calls.

Other thing you could look into is whether the manufactured constant names/scoping/prefix match what you get in MRI; it's equally likely that we're providing different names initially or that we're scoping constants wrong when defined in this way.

Member

headius commented Dec 5, 2017

@siutin Provide a simple executable example without spec and try to peel off any lines of code that aren't related. I can think of no reason why running inside an iterator/block would have any effect on these calls.

Other thing you could look into is whether the manufactured constant names/scoping/prefix match what you get in MRI; it's equally likely that we're providing different names initially or that we're scoping constants wrong when defined in this way.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 12, 2018

Member

No feedback since December, bumping.

Member

headius commented Feb 12, 2018

No feedback since December, bumping.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 12, 2018

Member

No feedback for four months, closing for now. If you can provide a simple reproduction, we can reopen. Or if you can't find a simple reproduction and this is still blocking you...we'll consider it 😄

Member

headius commented Apr 12, 2018

No feedback for four months, closing for now. If you can provide a simple reproduction, we can reopen. Or if you can't find a simple reproduction and this is still blocking you...we'll consider it 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment