Skip to content

Kernel#respond_to_missing? should return T::Boolean#7313

Merged
froydnj merged 1 commit intosorbet:masterfrom
LTe:respond-to-missing
Sep 15, 2023
Merged

Kernel#respond_to_missing? should return T::Boolean#7313
froydnj merged 1 commit intosorbet:masterfrom
LTe:respond-to-missing

Conversation

@LTe LTe requested a review from a team as a code owner September 15, 2023 13:48
@LTe LTe requested review from froydnj and removed request for a team September 15, 2023 13:48
@froydnj
Copy link
Copy Markdown
Contributor

froydnj commented Sep 15, 2023

Unfortunately, given:

https://github.com/ruby/ruby/blob/89802078f9f406be411032814e1960e62dbc7ce2/vm_method.c#L2852-L2867

I don't think we can guarantee that respond_to_missing will always return a boolean, since we can potentially call a user-implemented function without coercing the result to a boolean at line 2864. Contrast this to the coercion to a boolean at line 2866. (This is probably a bug in the Ruby VM, honestly, but it is what it is.)

Copy link
Copy Markdown
Contributor

@froydnj froydnj left a comment

Choose a reason for hiding this comment

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

Ah, sorry, I was confused about the previous bit.

@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented Sep 15, 2023

@froydnj yeah, it looks like you are right. But it is very strange, they should cast it into boolean I think.

@froydnj
Copy link
Copy Markdown
Contributor

froydnj commented Sep 15, 2023

We have a policy of testing changes to Sorbet against Stripe's codebase before
merging them. I've kicked off a test run for the current PR. When the build
finishes, I'll share with you whether or how it failed. Thanks!

Stripe employees can see the build results here:

https://go/builds/bui_Oe0ZC0Z03CpFET
https://go/builds/bui_Oe0ZgHIhafIjSb
https://go/builds/bui_Oe0ZJrKpnAfjTq

@froydnj froydnj merged commit 40ee234 into sorbet:master Sep 15, 2023
@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented Sep 16, 2023

About this one: https://github.com/ruby/ruby/blob/89802078f9f406be411032814e1960e62dbc7ce2/vm_method.c#L2852-L2867

Based on the code it looks like respond_to? can call respond_to_missing? and in case when respond_to_missing? will return something else than boolean then respond_to? will return the same.

Example

class Example
  def respond_to_missing?(*) = :sorbet
end

Example.new.respond_to?(:non_existing_method) => true

It looks like it is working just fine. There is also test in ruby for that

https://github.com/ruby/ruby/blob/89802078f9f406be411032814e1960e62dbc7ce2/spec/ruby/core/kernel/respond_to_missing_spec.rb#L48-L52

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.

2 participants