-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
src: remove unused private field #60802
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
Conversation
targos
commented
Nov 21, 2025
If there's a good reason to keep it, we should instead put a comment and disable compiler warnings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## main #60802 +/- ## ========================================== - Coverage 88.55% 88.54% -0.01% ========================================== Files 703 703 Lines 208259 208259 Branches 40162 40158 -4 ========================================== - Hits 184415 184400 -15 - Misses 15844 15887 +43 + Partials 8000 7972 -28
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This structure is a public API and changing the private fields will change the size of the struct and cause ABI incompatibility.
This field was named as reserved_.
This should be a semver-major change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok to keep it if it's reserved for something. Do you have a suggestion for an explaining comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling it void* reserved_; should be fine without a comment tbh -- this is a pretty common pattern in C/C++ APIs for ABI forward compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think renaming this field to reserved_ could be good. We can also remove this in a semver-major change.
Refs: #59828