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

Merge gc.h and internal/gc.h #7273

Merged
merged 1 commit into from Feb 9, 2023
Merged

Conversation

eightbitraptor
Copy link
Contributor

History:

Currently we have 4 headers named gc.h

  • gc.h
  • internal/gc.h
  • include/ruby/internal/gc.h
  • include/ruby/internal/intern/gc.h

The first two are private and internal to the CRuby codebase, the latter 2 are visible to C extensions. This PR attempts to merge the two internal headers gc.h and internal/gc.hinto a single headerinternal/gc.h` in order to simplify the codebase and put all the gc related interface in a single place.

Here is the history of the 2 internal only gc.h headers as I've been able to understand from the commit logs:

  • gc.h was created as part of the YARV merge in 2006, it contained only some defines for debug related macros.
  • In 2010 some function declarations were added to it to expose some functionality (rb_objspace_each_objects) to the objspace extension
  • In May 2011 internal.h was created as a place to put shared, but internal only declarations.
  • In June 2011 GC related functions started being committed to internal.h
  • In 2019 internal.h was refactored and internal/gc.h was created

As of 2023 internally used code continues to be added to both gc.h and internal/gc.h, which one is chosen in any particular circumstance looks to be whichever header is already being included for legacy reasons in the .c file being edited.

Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

There's several places in depend files that have $(top_srcdir)/gc.h, I think these can also go away.

compile.c Outdated Show resolved Hide resolved
misc/.vscode/settings.json Outdated Show resolved Hide resolved
@eightbitraptor eightbitraptor marked this pull request as ready for review February 9, 2023 13:34
@matzbot matzbot requested a review from a team February 9, 2023 13:34
yjit/src/cruby_bindings.inc.rs Show resolved Hide resolved
vm_sync.c Outdated Show resolved Hide resolved
string.c Outdated Show resolved Hide resolved
ractor.c Outdated Show resolved Hide resolved
mjit.c Outdated Show resolved Hide resolved
ext/objspace/objspace.c Outdated Show resolved Hide resolved
@eightbitraptor eightbitraptor force-pushed the mvh-remove-gc-h branch 2 times, most recently from 78c4ded to 8a19afc Compare February 9, 2023 14:21
@peterzhu2118 peterzhu2118 merged commit 72aba64 into ruby:master Feb 9, 2023
@peterzhu2118 peterzhu2118 deleted the mvh-remove-gc-h branch February 9, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants