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 RSymbol struct back into RVALUE #4378

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

peterzhu2118
Copy link
Member

Commit 0ca714f removed RSymbol from RVALUE. This commit adds RSymbol back into RVALUE.

@nobu can you confirm whether the removal was intentional or not?

Commit 0ca714f removed RSymbol from
RVALUE. This commit adds RSymbol back into RVALUE.
@peterzhu2118 peterzhu2118 requested a review from nobu April 12, 2021 19:37
gc.c Show resolved Hide resolved
@nobu
Copy link
Member

nobu commented Apr 13, 2021

I think it wasn't used at that time at least.

@peterzhu2118
Copy link
Member Author

peterzhu2118 commented Apr 13, 2021

Symbol is allocated in the GC as struct RSymbol so I think it should be in the union:

ruby/symbol.c

Line 611 in df7efdc

const VALUE dsym = rb_newobj_of(klass, T_SYMBOL | FL_WB_PROTECTED);

All the current places use the RSYMBOL macro to overcome this issue, but I think since it's allocated in the GC it should be in the union.

@peterzhu2118 peterzhu2118 merged commit 4eefb05 into ruby:master Apr 13, 2021
@peterzhu2118 peterzhu2118 deleted the pz-rvalue-symbol branch April 13, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants