Improve bad access error for std::expected<T, rfl::Error>

[hos-b]

This is a small cosmetic change that improves the error messages of std::bad_expected_access exceptions when the library is used with REFLECTCPP_USE_STD_EXPECTED.

I also wrote down a small test that verifies the exception type and message.

[gemini-code-assist]

Summary of Changes

Hello @hos-b, 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 refining the error handling experience within the rfl library when std::expected is utilized. By specializing the std::bad_expected_access exception for rfl::Error, the changes ensure that users receive clear and descriptive error messages, thereby improving debugging and overall robustness when an rfl::Result unexpectedly holds an error.

Highlights

• Improved Error Messages: The pull request enhances the error messages for std::bad_expected_access exceptions when REFLECTCPP_USE_STD_EXPECTED is enabled, making them more informative by providing the underlying rfl::Error message.
• Custom std::bad_expected_access Specialization: A specialization for std::bad_expected_access<rfl::Error> has been added in include/rfl/Result.hpp to override the what() method, ensuring the correct error string is returned.
• New Test Case: A new test file, tests/generic/test_bad_access_error.cpp, has been introduced to verify that the std::bad_expected_access<rfl::Error> exception is correctly thrown with the expected error message.

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.

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

This pull request aims to improve the error message when accessing the value of an rfl::Result that contains an error, specifically when using the std::expected-based implementation. This is achieved by specializing std::bad_expected_access<rfl::Error>. A test case is also added to verify the new behavior. While the goal is commendable, the current implementation of specializing std::bad_expected_access is ill-formed according to the C++ standard and will lead to undefined behavior. I have provided a critical review comment with a detailed explanation and a suggested alternative approach that is standard-compliant. The proposed alternative involves inheriting from std::expected rather than aliasing it, which allows for custom value() methods that can throw a std::runtime_error with the desired detailed message, unifying the behavior with the non-std::expected implementation.

[hos-b]

The cppreference page does not mention anything about the program being ill-formed in case of std::bad_expected_access specialization. The way it's worded, I'd even say it's expected that people would customize it:

Implementations are allowed but not required to override what().
bad_expected_access is the base class of all other bad_expected_access specializations.

Regarding the comment about constexpr, the move constructor of std::string has been constexpr since c++20.

[liuzicheng1987]

@hos-b thanks for the contribution! Sorry it took me a couple of days to merge it, I was a bit busy these days.

[hos-b]

@hos-b thanks for the contribution! Sorry it took me a couple of days to merge it, I was a bit busy these days.

No problem. Thank you for the review and happy holidays 🙂