Skip to content

Conversation

ssnickolay
Copy link
Collaborator

Inspired by https://github.com/palkan/ruby-compatibility-examples/blob/d36eae2d2ed3a76dd86172b28254e2be0b3ed56c/examples/refine_include_super.rb

When a method is called from Kernel we actually not execute Kernel.foo, but do it on new class based on Kernel (objectMetaClass #<Class:#<Module:0x28>>). In this case refinements are activated for Kernel do not work.
To fix it we need to go over objectMetaClass ancestors and take all proper refinements.

@ssnickolay ssnickolay force-pushed the feat/super-sees-ancestors-refinements branch 5 times, most recently from 308419f to f5f0871 Compare July 15, 2020 13:26
@ssnickolay ssnickolay force-pushed the feat/super-sees-ancestors-refinements branch from f5f0871 to 0705fb4 Compare July 15, 2020 13:27
@ssnickolay ssnickolay requested a review from eregon July 15, 2020 13:29
@ssnickolay ssnickolay marked this pull request as ready for review July 15, 2020 13:29
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Looks good!

@eregon eregon self-assigned this Jul 15, 2020
@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Jul 15, 2020
@eregon
Copy link
Member

eregon commented Jul 16, 2020

1 MRI test fails with this PR:

  1) Error:
TestRefinement#test_include_into_refinement:
SystemStackError: stack level too deep
	from TranslateExceptionNode.java:118:in `org.truffleruby.language.methods.TranslateExceptionNode.translateStackOverflow'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:737:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:729:in `baz'
    /b/b/e/main/test/mri/tests/ruby/test_refinement.rb:738:in `baz'
    ...

@ssnickolay
Copy link
Collaborator Author

ssnickolay commented Jul 16, 2020

Glad that we have this test. Fixed: d2c831b

@graalvmbot graalvmbot merged commit 0d78216 into oracle:master Jul 16, 2020
@eregon eregon added this to the 20.2.0 milestone Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants