Inline CPP functions in inl files

[thesecretluke]

Changelog

Add inline keywords to all definitions in C++ inl files

Docs

Description

None of the function definitions in the C++ .inl files had the "inline" qualifier. So I was including them with the headers, and getting many double-include errors. Adding inline to the function declarations fixes this, and should be more idiomatic C++ since these have the .inl extension.

Missing these isn't a huge deal for smaller projects but causes issues with the address sanitizer and linkers when this repo is a part of a library.

I tested this in a proprietary project that included the mcap/reader.hpp and mcap.writer.hpp in a module that was compiled to a shared library. It was built with CMake, using the target_include_directories on the mcap/cpp/mcap/include/mcap directory. That shared library was linked against a target. The linker also pulled a copy of the MCAP library into the binary, which led to a one-definition-rule violation.

Inlining these functions caused that issue to disappear, and also reduced binary size.

[defunctzombie]

So I was including them with the headers, and getting many double-include errors.

Were you including the implementation define in multiple cpp files or translation units? This define should only be in one cpp file (per the readme):

#define MCAP_IMPLEMENTATION  // Define this in exactly one .cpp file

[thesecretluke]

@defunctzombie

Yes, I only defined it once. The define was in a file that got compiled into the shared library (not the target that linked to that library), if that changes anything at all.

Note that without the inline, it compiles & runs fine, but only the address sanitizer fails.

But I do need to add the inline keywords to instead compile my shared library as a static library.

[james-rms]

The define was in a file that got compiled into the shared library (not the target that linked to that library), if that changes anything at all.

When you added the inline directive, you indicate to the compiler that there can be multiple definitions of these functions, but that's not the intention in this library. The intention is for the definitions to be compiled into whichever translation unit has #define MCAP_IMPLEMENTATION in it, and no other translation units.

That shared library was linked against a target. The linker also pulled a copy of the MCAP library into the binary, which led to a one-definition-rule violation.

I can't look into your build system to tell you where the problem is, but this error can only come up when you have the definitions in an .inl file getting included into more than one translation unit.

If it helps, you can create mcap.cpp in your project with these contents:

#define MCAP_IMPLEMENTATION
#include "mcap.hpp"

build it as its own shared library, and link it everywhere else to reduce the likelihood of confusion.