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

Remove FL_TEST and FL_SET #437

Merged
merged 1 commit into from Feb 24, 2022
Merged

Remove FL_TEST and FL_SET #437

merged 1 commit into from Feb 24, 2022

Conversation

tenderlove
Copy link
Contributor

Object flags are pretty internal information so we shouldn't be checking
them (if possible). In this case, we don't need to check the status of
EXIVAR because rb_copy_generic_ivar will check the flag and return if
there are no ivars to copy[1] and will set the flag on the cloned object
if it did copy[2]

Just for some background, FL_EXIVAR flag indicates that an object has
instance variables, but only objects that are not of the type
T_OBJECT, T_CLASS, or T_MODULE. This type of knowledge is really an
implementation detail, so I think C extensions should not know it or use
it.

  1. https://github.com/ruby/ruby/blob/24909f5b3de10b6f0b6f9acbf44ed347ccabb7e7/variable.c#L1725-L1727
  2. https://github.com/ruby/ruby/blob/24909f5b3de10b6f0b6f9acbf44ed347ccabb7e7/variable.c#L1741

Object flags are pretty internal information so we shouldn't be checking
them (if possible).  In this case, we don't need to check the status of
EXIVAR because `rb_copy_generic_ivar` will check the flag and return if
there are no ivars to copy[1] and will set the flag on the cloned object
if it did copy[2]

Just for some background, `FL_EXIVAR` flag indicates that an object has
instance variables, but only objects that are not of the type
`T_OBJECT`, `T_CLASS`, or `T_MODULE`.  This type of knowledge is really an
implementation detail, so I think C extensions should not know it or use
it.

1. https://github.com/ruby/ruby/blob/24909f5b3de10b6f0b6f9acbf44ed347ccabb7e7/variable.c#L1725-L1727
2. https://github.com/ruby/ruby/blob/24909f5b3de10b6f0b6f9acbf44ed347ccabb7e7/variable.c#L1741
@larskanis larskanis merged commit 76c4175 into ged:master Feb 24, 2022
@larskanis
Copy link
Collaborator

Make sense, thank you!

@tenderlove tenderlove deleted the rm-fl-test branch February 24, 2022 17:33
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.

None yet

2 participants