-
Notifications
You must be signed in to change notification settings - Fork 932
Add assertions and cleanup #582
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
3f23e19
to
5704bc2
Compare
@@ -354,7 +354,7 @@ inline static void rotateLog(AtomicBuffer& logMetaDataBuffer, std::int32_t curre | |||
LogBufferDescriptor::casActiveTermCount(logMetaDataBuffer, currentTermCount, nextTermCount); | |||
} | |||
|
|||
inline static void initializeTailWithTermId(AtomicBuffer& logMetaDataBuffer, int partitionIndex, std::int32_t termId) | |||
inline void initializeTailWithTermId(AtomicBuffer& logMetaDataBuffer, int partitionIndex, std::int32_t termId) |
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.
Why remove the static declaration from methods? Would this not be a breaking API 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.
It may break the ABI in case someone wants to compile object files with the new header and link with the client library that was compiled with the old headers. With a simple test case, I confirmed that two object files still link if one of them is inline static
and the other is only inline
. The advantage is that the resulting binary is smaller as the linker is now free to eliminate repeating inline
symbols as they are not internally linked.
e47382d
to
418fbcf
Compare
Can this be brought up to date? Right now it has conflicts. |
Only inline specifier is enough for functions in namespace scope in a header for avoiding multiple definitions.
const qualifier implies static in namespace scope.
418fbcf
to
4d5979c
Compare
Rebased on master. |
Related to #579