Add std::variant serialization support by hoshinohikari · Pull Request #13 · PavelKisliak/BitSerializer

hoshinohikari · 2026-03-28T07:50:21Z

No description provided.

Copilot

Pull request overview

Adds first-class std::variant support to BitSerializer and expands test coverage across unit + multiple archive integration suites, along with a few related build/tooling and serialization correctness tweaks.

Changes:

Introduce std::variant serialization as an object { index, value } and add unit/integration tests for it.
Extend test fixture generation to build std::variant values.
Adjust build/static-analysis CMake behavior and make small correctness fixes in supporting components (e.g., RapidJSON scope handling, UTF conversion, refine/noexcept, file path forwarding).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`include/bitserializer/types/std/variant.h`	New `std::variant` serialization implementation (`index` + `value`).
`tests/unit_tests/std_types_tests/std_types_tests.cpp`	Unit tests covering several `std::variant` alternatives + member usage.
`tests/integration_tests/rapidjson_archive_tests/rapidjson_archive_tests.cpp`	Integration tests for `std::variant` with RapidJSON archive.
`tests/integration_tests/rapidyaml_archive_tests/rapidyaml_archive_tests.cpp`	Integration tests for `std::variant` with RapidYAML archive.
`tests/integration_tests/pugixml_archive_tests/pugixml_archive_tests.cpp`	Integration tests for `std::variant` with PugiXML archive.
`tests/integration_tests/msgpack_archive_tests/msgpack_archive_tests.cpp`	Integration tests for `std::variant` with MsgPack archive.
`src/testing_tools/auto_fixture.h`	Add fixture builder for `std::variant`.
`include/bitserializer/rapidjson_archive.h`	Adjust key handling in save-mode scope opening (object/array).
`tests/unit_tests/CMakeLists.txt`	Temporarily disable clang-tidy while configuring unit test subdirs.
`tests/integration_tests/CMakeLists.txt`	Temporarily disable clang-tidy while configuring integration test subdirs.
`CMakeLists.txt`	Temporarily disable clang-tidy while configuring benchmarks/samples subdirs.
`src/testing_tools/common_test_entities.h`	Add NOLINT for value-param; minor formatting change.
`include/bitserializer/refine.h`	Tweak `noexcept` and assignability static_assert for fallback handling.
`include/bitserializer/conversion_detail/convert_utf.h`	Iterator/utility tweaks (includes + parameter passing + moves).
`include/bitserializer/bit_serializer.h`	Avoid forwarding `path` into APIs used multiple times (prevents moved-from issues).
`include/alloca.h`	Add Windows compatibility shim for `<alloca.h>`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T07:56:31Z

+			auto& insertedMember = FindMember(memberKey)->value;
+			return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMember, mAllocator, this->GetContext(), this, memberKey);


Same lifetime issue as OpenObjectScope: memberKey is a local string that is used to initialize parentKey (string_view) inside the returned array scope. This can leave mParentKey dangling and cause UB later (e.g., in GetPath()). Ensure the scope stores an owning key (or otherwise guarantees lifetime) before storing a string_view.

Suggested change

auto& insertedMember = FindMember(memberKey)->value;

return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMember, mAllocator, this->GetContext(), this, memberKey);

auto insertedMemberIt = FindMember(memberKey);

auto& insertedMemberValue = insertedMemberIt->value;

const auto* insertedMemberKey = insertedMemberIt->name.GetString();

return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMemberValue, mAllocator, this->GetContext(), this, insertedMemberKey);

Copilot · 2026-03-28T07:56:32Z

 public:
 	TestClassWithSubTypes() = default;
-	TestClassWithSubTypes(Args... args)
+		TestClassWithSubTypes(Args... args) // NOLINT(performance-unnecessary-value-param)


This constructor is indented inconsistently compared to the surrounding methods (extra leading tab), which looks accidental and makes the class harder to read. Align it with the other members’ indentation.

Suggested change

TestClassWithSubTypes(Args... args) // NOLINT(performance-unnecessary-value-param)

TestClassWithSubTypes(Args... args) // NOLINT(performance-unnecessary-value-param)

Copilot · 2026-03-28T07:56:32Z

+					if (value.index() != TIndex)
+					{
+						value.template emplace<TIndex>();
+					}
+					Serialize(archive, key, std::get<TIndex>(value));


When loading, this code switches the active alternative via value.emplace<TIndex>() without constructor args. That silently imposes a requirement that every alternative that might be loaded is default-constructible; otherwise compilation fails with a hard-to-diagnose error. Consider adding a clear static_assert documenting this requirement (or refactoring to deserialize into a temporary and only emplace/assign on success, if you want to support non-default-constructible alternatives).

Copilot · 2026-03-28T07:56:32Z

+			else if (archive.GetOptions().mismatchedTypesPolicy == MismatchedTypesPolicy::ThrowError)
+			{
+				throw SerializationException(SerializationErrorCode::MismatchedTypes,
+					"Variant index is out of range for target type: " + Convert::ToString(activeIndex));
+			}


New behavior: when loading and mismatchedTypesPolicy == ThrowError, an out-of-range variant index throws SerializationException(MismatchedTypes, ...). There are tests for mismatched tuple sizes, but no tests added here to assert this variant-specific error path. Add a unit/integration test that loads a variant with an invalid index and verifies the exception behavior (and optionally a Skip-policy case).

Copilot · 2026-03-28T07:56:33Z

@@ -0,0 +1,13 @@
+/*******************************************************************************


Most headers under include/ appear to use the standard BitSerializer copyright/MIT license banner at the top. This new compatibility header doesn’t include that banner, which makes licensing/attribution inconsistent—consider adding the standard header block here too.

Suggested change

/*******************************************************************************

/*******************************************************************************

* BitSerializer - Core serialization library *

* *

* Copyright (c) BitSerializer contributors *

* Distributed under the MIT license. See LICENSE file in the project root for *

* full license information. *

*******************************************************************************/

/*******************************************************************************

Copilot · 2026-03-28T07:56:33Z

+			const auto memberKey = Convert::To<key_type>(key);
+			SaveJsonValue(memberKey, RapidJsonNode(rapidjson::kObjectType));
+			auto& insertedMember = FindMember(memberKey)->value;
+			return std::make_optional<RapidJsonObjectScope<TMode, TEncoding, TAllocator>>(&insertedMember, mAllocator, this->GetContext(), this, memberKey);


In save mode, memberKey is a local key_type and is passed to the child scope constructor as parentKey (a std::string_view). The returned scope stores this view (mParentKey), so it will dangle after OpenObjectScope returns, leading to UB when building paths / error messages. Pass a non-dangling view (e.g., derived directly from the original key input when it has stable storage) or make the scope own a copy of the key and store a view into that owned storage.

PavelKisliak · 2026-03-29T15:13:28Z

Hi @hoshinohikari,

Thank you so much for this PR! The serialization of std::variant is really important part which still missing in the library.
I originally think to implement std::variant serialization without any metadata (this is the main philosophy of the library), but that would require significant effort for support by all archives.

Since serializing std::variant without metadata would likely involve some tradeoffs, I think having a tagged version (as you suggest) would be nice too. In order to leave room for implementing serialization without metadata, I propose adding a wrapper class VariantAsTagged:

template <typename T>
class VariantAsTagged
{
public:
    explicit VariantAsTagged(T& v) : value(v) {}
    T& value;
};

template <typename TArchive, typename... TArgs>
void SerializeObject(TArchive& archive, VariantAsTagged<std::variant<TArgs...>>& value)
{

}

In this case, the usage would look like this:

std::variant<int, std::string, CUser> data;
archive << KeyValue("data", VariantAsTagged(data));

(This is the same pattern as EnumAsBin or CTimeRef).

PavelKisliak · 2026-03-29T15:18:19Z

+if(STATIC_ANALYSIS_CLANG_TIDY)
+    set(_BITSERIALIZER_SAVED_CXX_CLANG_TIDY "${CMAKE_CXX_CLANG_TIDY}")
+    set(CMAKE_CXX_CLANG_TIDY "")
+endif()


Since the core of BitSerializer is a header-only, the test files are the primary place where template code gets instantiated and analyzed. Please use inline suppressions for false positives.

PavelKisliak · 2026-03-29T15:27:59Z

@@ -0,0 +1,13 @@
+/*******************************************************************************


As far as I understand this was added to fix issue with compiling RapidYaml archive on clang-cl, looks like this is already addressed by author.

PavelKisliak · 2026-03-30T12:21:13Z

+namespace AutoFixtureDetail
+{
+	template <size_t TIndex = 0, typename... TArgs>
+	void BuildVariantFixture(std::variant<TArgs...>& value, size_t activeIndex)
+	{
+		if constexpr (TIndex < sizeof...(TArgs))
+		{
+			if (activeIndex == TIndex)
+			{
+				value.template emplace<TIndex>();
+				::BuildFixture(std::get<TIndex>(value));
+			}
+			else
+			{
+				BuildVariantFixture<TIndex + 1>(value, activeIndex);
+			}
+		}
+	}
+}
+
+/**
+ * @brief Builds a test fixture for `std::variant` value.
+ *
+ * Randomly selects one of the alternatives and initializes it.
+ */
+template <typename ...TArgs>
+[[maybe_unused]] static void BuildFixture(std::variant<TArgs...>& value)
+{
+	static_assert(sizeof...(TArgs) > 0);
+	const size_t activeIndex = static_cast<size_t>(std::rand()) % sizeof...(TArgs);
+	AutoFixtureDetail::BuildVariantFixture(value, activeIndex);
+}
+


Currently CI fails due to unknown override of BuildFixture(), you can fix it by moving this code to the end of file.

hoshinohikari added 2 commits March 27, 2026 22:15

feat: add std::variant serialization support

c4f33c3

fix: stabilize clang-tidy and build compatibility

b05ccbd

Copilot AI review requested due to automatic review settings March 28, 2026 07:50

Copilot started reviewing on behalf of hoshinohikari March 28, 2026 07:50 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

PavelKisliak reviewed Mar 29, 2026

View reviewed changes

PavelKisliak reviewed Mar 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add std::variant serialization support#13

Add std::variant serialization support#13
hoshinohikari wants to merge 2 commits intoPavelKisliak:masterfrom
hoshinohikari:master

hoshinohikari commented Mar 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

PavelKisliak commented Mar 29, 2026 •

edited

Loading

Uh oh!

PavelKisliak Mar 29, 2026

Uh oh!

PavelKisliak Mar 29, 2026

Uh oh!

PavelKisliak Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		auto& insertedMember = FindMember(memberKey)->value;
		return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMember, mAllocator, this->GetContext(), this, memberKey);

-			auto& insertedMember = FindMember(memberKey)->value;
-			return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMember, mAllocator, this->GetContext(), this, memberKey);
+			auto insertedMemberIt = FindMember(memberKey);
+			auto& insertedMemberValue = insertedMemberIt->value;
+			const auto* insertedMemberKey = insertedMemberIt->name.GetString();
+			return std::make_optional<RapidJsonArrayScope<TMode, TEncoding, TAllocator>>(&insertedMemberValue, mAllocator, this->GetContext(), this, insertedMemberKey);

	TestClassWithSubTypes(Args... args) // NOLINT(performance-unnecessary-value-param)
	TestClassWithSubTypes(Args... args) // NOLINT(performance-unnecessary-value-param)

		@@ -0,0 +1,13 @@
		/*******************************************************************************

-/*******************************************************************************
+/*******************************************************************************
+* BitSerializer - Core serialization library                                   *
+*                                                                              *
+* Copyright (c) BitSerializer contributors                                     *
+* Distributed under the MIT license. See LICENSE file in the project root for  *
+* full license information.                                                    *
+*******************************************************************************/
+/*******************************************************************************

Conversation

hoshinohikari commented Mar 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

PavelKisliak commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PavelKisliak Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

PavelKisliak Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

PavelKisliak Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PavelKisliak commented Mar 29, 2026 •

edited

Loading