Made sure that apply can never move or modify the tuple values; fixes…

[liuzicheng1987]

… #439

[gemini-code-assist]

Hello @liuzicheng1987, 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!

Summary of Changes

Hello team,

Gemini here, providing a summary of this pull request authored by liuzicheng1987.

The primary goal of this PR, as indicated by the title and description, is to address issue #439 by ensuring that the apply method on rfl::NamedTuple cannot be used to modify or move the underlying field values. This promotes const correctness and prevents unintended side effects when iterating over tuple elements.

The core change involves removing the non-const overload of the apply method, leaving only the const version. The documentation and a new test case have been updated to reflect and verify this change.

Highlights

• Preventing Tuple Modification: The non-const overload of the apply method for rfl::NamedTuple has been removed. This guarantees that any function applied using NamedTuple::apply will receive const references to the fields, preventing modification or moving of the tuple's contents.
• Documentation Update: The documentation for NamedTuple::apply in docs/named_tuple.md has been updated. An example that previously showed how to obtain a non-const pointer to a field value (which is now impossible via apply) has been removed. The remaining example correctly demonstrates accessing field values using const auto&.
• New Test Case: A new test file, tests/json/test_apply_on_named_tuple.cpp, has been added. This test specifically verifies the usage of the apply method on a NamedTuple and confirms that the tuple's state remains unchanged after applying a function, by checking its JSON serialization.

Changelog

• docs/named_tuple.md
  • Removed documentation example demonstrating modification of field values via apply.
  • Updated the remaining example to correctly use const auto& for accessing field values.
• include/rfl/NamedTuple.hpp
  • Removed the non-const overload of the apply method for NamedTuple.
  • Changed the signature of the remaining const apply method to const&.
• tests/json/test_apply_on_named_tuple.cpp
  • Added a new test file to specifically test the apply method on NamedTuple.
  • The test verifies that apply can be used to access field values and that the tuple's state remains unchanged.

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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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 is currently in preview and 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 to provide feedback.

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.

---

Const correctness sought,
Tuple values safe and sound,
Apply, but don't change.

Footnotes

1. 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

The pull request successfully addresses issue #439 by ensuring that the apply method on rfl::NamedTuple cannot be used to modify the underlying field values. This is achieved by removing the non-const overload of apply and updating the documentation and tests accordingly. The changes are well-aligned with the stated objective.

Summary of Findings

• Prevention of modification via apply: The non-const overload of the apply method was removed, and the remaining const overload was ref-qualified with const&. This effectively prevents modification of the NamedTuple's fields when using apply, addressing the issue mentioned in the PR title.
• Documentation and Test Updates: The documentation (docs/named_tuple.md) and a new test case (tests/json/test_apply_on_named_tuple.cpp) were updated/added to reflect the change in the apply method's behavior and signature. This ensures consistency and provides coverage for the modified functionality.
• Severity Filtering: Based on the review settings, only issues with severity medium, high, or critical were included as specific review comments. No issues of these severity levels were found in the changes. Any potential low severity issues (such as minor style guide points, though none were obvious in the diffs) were not commented on directly.

Merge Readiness

The code changes appear correct and implement the intended fix. The documentation and tests have been updated to reflect the changes. As no medium, high, or critical severity issues were found during the review, the pull request seems ready to be merged. Please note that I am unable to directly approve the pull request; other reviewers should review and approve this code before merging.

[lgpseu]

@liuzicheng1987 make_fields brings unecessary field value copies. I think apply should avoid it as a visitor pattern function. rfl::to_view before apply maybe a choice, but it will bring interface change.

[liuzicheng1987]

@lgpseu at the moment, I don't really see another way. To avoid this, we would have to be able to store references, which is very tricky.

You can always use to_view(...).apply(...), which is how this was meant to be used anyway.