Skip to content
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

Add Data.define #3341

Merged
merged 7 commits into from
Jan 16, 2024
Merged

Add Data.define #3341

merged 7 commits into from
Jan 16, 2024

Conversation

moste00
Copy link
Contributor

@moste00 moste00 commented Nov 29, 2023

Adds a new core class to represent simple immutable value object. The class is
similar to Struct and partially shares an implementation, but has more
lean and strict API. See this redmine discussion for more.

Still in an incomplete state.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 29, 2023
@moste00 moste00 mentioned this pull request Nov 29, 2023
87 tasks
@eregon
Copy link
Member

eregon commented Nov 30, 2023

Thank you, I'll take it from here.

@eregon eregon self-assigned this Nov 30, 2023
@eregon eregon force-pushed the compat/Data.define branch 2 times, most recently from f91a411 to 86be023 Compare November 30, 2023 15:44
@eregon eregon marked this pull request as ready for review November 30, 2023 15:44
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.

It's passing both specs and MRI tests now, so should be good to go.

@@ -419,7 +419,7 @@ def pretty_print_cycle(q) # :nodoc:
class Data # :nodoc:
def pretty_print(q) # :nodoc:
q.group(1, sprintf("#<data %s", PP.mcall(self, Kernel, :class).name), '>') {
q.seplist(PP.mcall(self, Data, :members), lambda { q.text "," }) {|member|
q.seplist(PP.mcall(self, Kernel, :class).members, lambda { q.text "," }) {|member|
Copy link
Member

Choose a reason for hiding this comment

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

This is because we don't define Data#members but define it on the instance_methods_module.
I don't think it makes much sense to define it on Data as it would be a megamorphic call to members on the actual object's class.
So rather I think we should upstream this change.

Copy link
Member

Choose a reason for hiding this comment

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

@eregon eregon force-pushed the compat/Data.define branch 2 times, most recently from a67180e to fb54b5e Compare November 30, 2023 16:44
@eregon eregon force-pushed the compat/Data.define branch from 25437f5 to 0edc91b Compare January 11, 2024 13:34
@graalvmbot graalvmbot merged commit e4f49f0 into oracle:master Jan 16, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants