Skip to content

Allow overriding builtin methods#15

Merged
marcandre merged 6 commits into
masterfrom
udpate
Sep 14, 2020
Merged

Allow overriding builtin methods#15
marcandre merged 6 commits into
masterfrom
udpate

Conversation

@marcandre
Copy link
Copy Markdown
Member

@marcandre marcandre commented Sep 8, 2020

Details on ruby-lang; probably best to discuss this there.

@marcandre marcandre force-pushed the udpate branch 3 times, most recently from 2828baa to 66ac661 Compare September 8, 2020 22:30
@Hanmac
Copy link
Copy Markdown

Hanmac commented Sep 9, 2020

i have seen ruby making problems when using method_missing without respond_to_missing so it might cause problems there

@marcandre
Copy link
Copy Markdown
Member Author

i have seen ruby making problems when using method_missing without respond_to_missing so it might cause problems there

The respond_to_missing was added only for lazy initialization, which I'm removing. Otherwise it has always been standard behavior that:

os = OpenStruct.new
os.respond_to?(:foo) # => false
os.foo # => nil

That's in part because responding to :to_ary, :to_str, etc., could be problematic.

Comment thread test/ostruct/test_ostruct.rb Outdated
end

def test_frozen
return if RUBY_ENGINE == 'truffleruby' # https://github.com/oracle/truffleruby/issues/2093
Copy link
Copy Markdown
Member

@eregon eregon Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this has been fixed by @bjfish in truffleruby/truffleruby#2093 and the CI uses truffleruby-head, it should work fine without this change, @marcandre can you try?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@marcandre marcandre merged commit 6403210 into master Sep 14, 2020
@knu knu deleted the udpate branch December 22, 2020 14:33
@pocke pocke mentioned this pull request Aug 6, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants