Skip to content

Improve superclass retrieval API - #933

Merged
st0012 merged 4 commits into
mainfrom
codex/model-basic-object-parent
Jul 16, 2026
Merged

Improve superclass retrieval API#933
st0012 merged 4 commits into
mainfrom
codex/model-basic-object-parent

Conversation

@st0012

@st0012 st0012 commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member
  • Let's call it get_superclass instead, to match Ruby's superclass API name
  • get_superclass should handle Object and BasicObject's superclass correctly
  • Caller of get_superclass should not need to pass definitions separately

@st0012
Model BasicObject as a parentless class
d7f93bf 
Assisted-By: devx/7a36e49d-f7b5-4912-8df7-4798f1f95608
@st0012
st0012 requested a review from a team as a code owner July 15, 2026 18:07
@st0012
Make superclass lookup declaration-centric
a9c3b4e 
Assisted-By: devx/7a36e49d-f7b5-4912-8df7-4798f1f95608
@st0012 st0012 changed the title Model BasicObject as a parentless class Improve superclass retrieval API Jul 15, 2026
@st0012 st0012 self-assigned this Jul 15, 2026
@st0012 st0012 added the chore Something that should not be included in release notes label Jul 15, 2026
@st0012
Rename superclass hierarchy test
96166a4 
Assisted-By: devx/7a36e49d-f7b5-4912-8df7-4798f1f95608
vinistock
vinistock reviewed Jul 15, 2026
View reviewed changes

@vinistock vinistock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand the motivation of this change. Don't we already handle object and basic object superclasses correctly in main?

@st0012

st0012 commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

I was reviewing #893 and noticed that get_parent_class didn't handle the BasicObject case correctly when reading here. I know behaviour-wise it's handled in linearize_parent_ancestors, but IMO get_parent_class should already handle that case.

vinistock
vinistock reviewed Jul 16, 2026
View reviewed changes

@vinistock vinistock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The refactor is good.

Did you benchmark to see if this provides any speed ups?

Comment thread rust/rubydex/src/resolution_tests.rs Outdated
Comment thread rust/rubydex/src/resolution_tests.rs Outdated
Comment thread rust/rubydex/src/resolution.rs
@st0012
st0012 requested a review from vinistock July 16, 2026 17:06
vinistock
vinistock approved these changes Jul 16, 2026
View reviewed changes
Comment thread rust/rubydex/src/resolution.rs
@st0012
st0012 merged commit d2f2e8a into main Jul 16, 2026
36 checks passed
@st0012
st0012 deleted the codex/model-basic-object-parent branch July 16, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinistock vinistock vinistock approved these changes

Assignees

@st0012 st0012

Labels

chore Something that should not be included in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@st0012 @vinistock