USCiLab / cereal

A C++11 library for serialization
BSD 3-Clause "New" or "Revised" License
4.18k stars 751 forks source link

Embedded RapidJSON can conflict with another RapidJSON used by the client (ODR violation!) #800

Open aurelienrb opened 1 year ago

aurelienrb commented 1 year ago

Hello,

Cereal is embedding its own version of RapidJSON. This embedded version has been patched to use custom include path (cereal/external/rapidjson/ instead of rapidjson/) and macros ( CEREAL_RAPIDJSON_ instead of RAPIDJSON_).

This prevents a conflict at compile time with another RapidJSON used by the client.

But! The RapidJSON symbols are still defined in the same original namespace rapidjson: https://github.com/USCiLab/cereal/blob/v1.3.2/include/cereal/external/rapidjson/rapidjson.h#L118

#define CEREAL_RAPIDJSON_NAMESPACE rapidjson

As a result, if a client uses both Cereal and a different version of RapidJSON (or compiles it with an option such as RAPIDJSON_HAS_STDSTRING) then (s)he ends up with an ODR violation 😢

That's what happened to me: my application works well on Windows 11 and crashes on Windows 10... ODR magic!

I see several options here, that could be combined:

I selected the second option. And at the same time, I removed the (forced) dependency on RapidXML because I don't want to embed dependencies I don"t need (I am in a strongly regulated context).

Note that, in my case, all of that is done by a Conan recipe that patches and cleans up Cereal code before packaging it.

What do you think is the best option?