Cleanup includes #515

KostinPavel · 2025-10-10T14:07:28Z

No description provided.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…ast.hpp и неиспользуемые include из Result.hpp - Удален #include <bit> из include/rfl/internal/ptr_cast.hpp (не используется в коде) - Удалены неиспользуемые include из include/rfl/Result.hpp: - #include <algorithm> (не используется) - #include <iostream> (не используется) - #include <optional> (не используется)

Этап 2: Базовые типы (средний риск) Удалены неиспользуемые include из файла Tuple.hpp: - <algorithm> - не используется в коде - <bit> - не используется в коде - <cstdint> - не используется в коде - <limits> - не используется в коде - <memory> - не используется в коде - <optional> - не используется в коде Оставлены только необходимые заголовки: - <array> - используется для std::array - <tuple> - используется для std::tuple - <type_traits> - используется для различных type traits - <utility> - используется для std::integer_sequence и других утилит Все остальные файлы (Literal.hpp, Field.hpp, Object.hpp, Variant.hpp) уже имели оптимальный набор include файлов без неиспользуемых зависимостей. Компиляция проекта проверена и проходит успешно.

…вки <string>, <version>, упрощен оператор сравнения spaceship

unused arguments shaded variables

gemini-code-assist · 2025-10-10T14:12:13Z

Summary of Changes

Hello @KostinPavel, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on a comprehensive cleanup and enhancement of the reflect-cpp library. It introduces new code quality checks, refines build configurations, and drastically reduces unnecessary header includes across the codebase. These changes aim to improve compilation efficiency, reduce binary size, and enhance the overall robustness and maintainability of the serialization backends.

Highlights

Code Quality Tooling: A new .clang-tidy configuration file has been added, enabling several checks such as misc-unused-using-decls, misc-include-cleaner, modernize-use-using, readability-duplicate-include, and readability-redundant-preprocessor to enforce better code hygiene.
Build System Enhancements: The CMakeLists.txt file was updated to enable CMAKE_EXPORT_COMPILE_COMMANDS for improved tooling integration and expanded compile options for the reflectcpp target, including more stringent warnings for debug builds and explicit NDEBUG for release builds.
Extensive Include Cleanup: Numerous unnecessary standard library headers (e.g., <array>, <iostream>, <optional>, <type_traits>, <algorithm>, <tuple>) have been removed from various rfl library headers and benchmark files across different serialization backends (Avro, BSON, Cap'n Proto, CBOR, CSV, Flexbuf, Generic, JSON, Msgpack, Parquet, TOML, UBJSON, YAML). This significantly reduces compilation dependencies and improves build times.
Serialization Writer Robustness: Safety improvements were implemented in the Avro and Cap'n Proto writers. The Avro writer now includes checks for return values of avro_value_ functions and null checks for string data. The Cap'n Proto writer introduces a SafeSizeToUInt helper to prevent size_t to unsigned int overflow during list initialization and indexing.
Schema Default Initialization: Struct members within the Avro and JSON schema definition files (include/rfl/avro/schema/Type.hpp and include/rfl/json/schema/Type.hpp) are now consistently default-initialized using {}.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This is a great cleanup pull request that improves code hygiene by removing unused includes and strengthens the build process with more compiler warnings and clang-tidy integration. The added error and null checks in rfl/avro/Writer.hpp are very important for robustness. However, I've found some critical issues with the current error handling within noexcept functions, which could lead to silent data corruption or loss. I've left specific comments on these areas. I've also made a suggestion to improve the std::hash specialization for rfl::Ref. Lastly, I noticed several headers are commented out (e.g., //#include ...). If these are no longer needed, it would be cleaner to remove them entirely. If they are needed for specific build configurations, using preprocessor guards (#ifdef ...) would be a more robust solution.

gemini-code-assist · 2025-10-10T14:16:37Z

include/rfl/avro/Writer.hpp

+    int result = avro_value_append(&_parent->val_, &new_value, nullptr);
+    if (result != 0) {
+      return OutputVarType{_parent->val_};
+    }


The error handling for avro_value_append appears to be incorrect. When result != 0, an error has occurred, but the function returns OutputVarType{_parent->val_}. This returns the parent container as if it were the newly added value, which is misleading and can lead to silent data corruption. The caller has no way of knowing that the append operation failed because the function is marked noexcept.

This pattern is repeated in add_value_to_map, add_value_to_object, and add_value_to_union.

A noexcept function that encounters an error it cannot handle should either terminate (e.g., std::terminate()) or have a mechanism to signal the error state to the caller without exceptions. Simply returning an incorrect value silently is dangerous.

Consider changing the write pipeline to propagate errors, perhaps by removing noexcept and returning a rfl::Result. If noexcept must be preserved, the Writer could maintain an internal error state that can be checked after the write operation is complete.

what code will solve the problem?

@KostinPavel , handling these errors properly would require us to get rid of all of the noexcept and start throwing exceptions. It would make sense in my view, because these would truly be exceptional circumstances (such as running out of disc space or memory).

Are exceptions allowed?
example: throw std::runtime_error(std::format("avro_value_append error: {:s}", avro_strerror()));

@KostinPavel , sure. Just remove all the noexcepts so this won't result in a crash.

@KostinPavel , by the way, I am not sure that std::format is supported by all the compiler versions we test for. I think some of the older versions don't support it yet.

gemini-code-assist · 2025-10-10T14:16:37Z

include/rfl/avro/Writer.hpp

+      const char* cstr = _var.c_str();
+      if (!cstr) {
+        return;
+      }
+      avro_value_set_string_len(_val, cstr, _var.size() + 1);


The null check for _var.c_str() results in a silent return, causing data to be dropped without any notification to the caller. This can lead to silent data loss, which can be very difficult to debug. The same issue exists for rfl::Bytestring and rfl::Vectorstring on lines 206-208.

Since this function is noexcept, you cannot throw an exception. Consider one of the following approaches:

Change the write pipeline to allow for error propagation (e.g., by removing noexcept and returning rfl::Result).

If noexcept is a hard requirement, have the Writer class maintain an error state that can be checked after the operation.

As a last resort, log the error before returning to make the silent failure observable.

@KostinPavel , why do we need this check? How would std::string.c_str() ever be a nullptr?

Its my bug.

No, checking for nullptr in this line is not necessary. That's why:
Guarantees of std::string::c_str():
The c_str() method for non-null std::string always returns a valid pointer to a null-terminated string.
Even for an empty string, c_str() returns a pointer to a valid null byte ('\0'), not nullptr.
Compilation condition:
The code is executed only when T is std::string (checking std::is_same<std::remove_cvref_t, std::string>())
This ensures that _var is indeed an std::string and has a valid c_str()

include/rfl/Ref.hpp

liuzicheng1987 · 2025-10-10T19:28:29Z

@KostinPavel , excellent work, thank you so much!

KostinPavel · 2025-10-10T19:43:22Z

This is for everyone's benefit. I just removed the unnecessary stuff. I didn't ruin anything. We're all waiting for C++26.

liuzicheng1987 · 2025-10-10T20:19:11Z

@KostinPavel , yeah, when C++ 26 is out, I will refactor this library to support both the C++ 26 reflection features, but continue supporting the old C++ 20 approaches.

KostinPavel and others added 12 commits October 8, 2025 15:22

Update write.hpp

1e87454

Update Writer.hpp

ab504ff

Update Type.hpp

a5e19b7

Update JSONSchema.hpp

192cb83

Update JSONSchema.hpp

4e58d12

Update Type.hpp

95cd0d4

Update include/rfl/json/schema/Type.hpp

d647307

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

not required includes 1

3a3a4e5

Очистка include файлов в Validator.hpp: удалены неиспользуемые заголо…

bc1ba35

…вки <string>, <version>, упрощен оператор сравнения spaceship

only necessary includes

ee7dfe2

unused arguments shaded variables

gemini-code-assist bot reviewed Oct 10, 2025

View reviewed changes

This was referenced Oct 10, 2025

Silent errors, can terminate application #516

Open

The null check for _var.c_str() results in a silent return, causing data to be dropped without any notification to the caller #517

Closed

KostinPavel and others added 2 commits October 10, 2025 23:45

Update Writer.hpp

9732c2c

Update Refl.hpp

bcf88d1

Cleanup includes #515

Are you sure you want to change the base?

Cleanup includes #515

Conversation

KostinPavel commented Oct 10, 2025

Uh oh!

gemini-code-assist bot commented Oct 10, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

KostinPavel Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

liuzicheng1987 Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

KostinPavel Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuzicheng1987 Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuzicheng1987 Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

liuzicheng1987 Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

KostinPavel Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liuzicheng1987 commented Oct 10, 2025

Uh oh!

KostinPavel commented Oct 10, 2025

Uh oh!

liuzicheng1987 commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KostinPavel Oct 10, 2025 •

edited

Loading

liuzicheng1987 Oct 11, 2025 •

edited

Loading

KostinPavel Oct 10, 2025 •

edited

Loading