feat(error_handling): Add Result<T, E> alias and macros to enable ergonomic Rust-style error handling.
#47
Conversation
WalkthroughThis pull request introduces a new public header, Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bill-hbrhbr
changed the title
Result<T> alias and macros for Rust-like error handling.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/ystdlib/error_handling/Result.hpp (1)
21-34: Consider adding more comprehensive error checking in theYSTDLIB_TRYmacro.The macro works well for propagating errors, similar to Rust's
?operator. However, it might be beneficial to add more compile-time checks or documentation regarding potential pitfalls.#define YSTDLIB_TRY(expr) \ ({ \ auto result{(expr)}; \ if (result.has_error()) { \ return result.error(); \ } \ using ReturnType = decltype(result.value()); \ - static_assert(std::is_move_constructible_v<ReturnType>); \ + static_assert(std::is_move_constructible_v<ReturnType>, \ + "The return type must be move constructible to use YSTDLIB_TRY"); \ std::move(result.value()); \ })src/ystdlib/error_handling/test/test_Result.cpp (5)
32-40: Preferfalse == result_no_error.has_error()instead ofREQUIRE_FALSE.According to the coding guidelines, we should prefer
false == <expression>rather than!<expression>orREQUIRE_FALSE.auto const result_no_error{cVoidFunc(false)}; - REQUIRE_FALSE(result_no_error.has_error()); + REQUIRE(false == result_no_error.has_error()); REQUIRE(std::is_void_v<decltype(result_no_error.value())>);
42-54: Ensure consistent comparison style in line 53.The comparison on line 53 uses parentheses that are not needed elsewhere. Also, consider following the coding guideline for
false ==comparison style in line 48.auto const main_no_error{main_func(false)}; - REQUIRE_FALSE(main_no_error.has_error()); + REQUIRE(false == main_no_error.has_error()); REQUIRE(std::is_void_v<decltype(main_no_error.value())>); auto const main_has_error{main_func(true)}; REQUIRE(main_has_error.has_error()); - REQUIRE((BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error())); + REQUIRE(BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error());The static analysis hint about a syntax error on line 42 appears to be a false positive. The test case declaration is correctly formatted.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 42-42: syntax error
(syntaxError)
56-64: Remove unnecessary parentheses in line 59 and apply consistent style.Following the pattern in the rest of the file, the comparison on line 59 should not need extra parentheses. Also, consider using the coding standard's preferred comparison style.
auto const result_no_error{cIntFunc(false)}; - REQUIRE_FALSE(result_no_error.has_error()); - REQUIRE((cTestInt == result_no_error.value())); + REQUIRE(false == result_no_error.has_error()); + REQUIRE(cTestInt == result_no_error.value());
66-78: Apply consistent style to comparisons.The same pattern of unnecessary parentheses and the use of
REQUIRE_FALSEshould be addressed for consistency.auto const main_no_error{main_func(false)}; - REQUIRE_FALSE(main_no_error.has_error()); + REQUIRE(false == main_no_error.has_error()); REQUIRE(std::is_void_v<decltype(main_no_error.value())>); auto const main_has_error{main_func(true)}; REQUIRE(main_has_error.has_error()); - REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
80-89: Apply consistent style to the final test case.For consistency with the suggested changes in the previous test cases, update the style here as well.
auto main_func = [&](bool is_error) -> Result<int> { return YSTDLIB_TRY(cIntFunc(is_error)); }; auto const main_no_error{main_func(false)}; - REQUIRE_FALSE(main_no_error.has_error()); - REQUIRE((cTestInt == main_no_error.value())); + REQUIRE(false == main_no_error.has_error()); + REQUIRE(cTestInt == main_no_error.value()); auto const main_has_error{main_func(true)}; REQUIRE(main_has_error.has_error()); - REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/ystdlib/error_handling/CMakeLists.txt(1 hunks)src/ystdlib/error_handling/Result.hpp(1 hunks)src/ystdlib/error_handling/test/test_Result.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/error_handling/Result.hppsrc/ystdlib/error_handling/test/test_Result.cpp
🧬 Code Definitions (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
src/ystdlib/error_handling/Result.hpp (1)
success(17-17)
🪛 Cppcheck (2.10-2)
src/ystdlib/error_handling/test/test_Result.cpp
[error] 42-42: syntax error
(syntaxError)
🔇 Additional comments (7)
src/ystdlib/error_handling/CMakeLists.txt (3)
6-6: LGTM! Added the new public header file.The addition of
Result.hppto the public headers looks good. This aligns with the PR's objective to introduce a type alias and macros for Rust-like error handling.
7-8: LGTM! Added the new public link library.The addition of
outcome::hlas a public link library is appropriate since it's required for the implementation of theResulttype alias and related functionality.
11-12: LGTM! Added the test file for the new functionality.The inclusion of
test/test_Result.cppin the test sources ensures proper testing of the newResultfunctionality. Good practice to add tests alongside new features.src/ystdlib/error_handling/Result.hpp (3)
11-12: LGTM! Well-designed template alias for error handling.The
Resulttemplate alias withstd::error_codeas the default error type is an elegant solution that aligns with Rust's approach while leveraging C++ standards. This implementation enables seamless integration with existingystdlib::error_handling::ErrorCodetypes that implicitly convert tostd::error_code.
14-19: LGTM! Thesuccess()function enhances readability.The
success()function is appropriately marked with[[nodiscard]]to prevent accidental misuse, and its implementation correctly delegates to the underlying OUTCOME_V2_NAMESPACE functionality. This helper function makes the code more readable when working withResult<void>functions.
36-46: LGTM! TheYSTDLIB_ASSERT_NO_ERRORmacro simplifies error handling for void returns.The macro correctly uses modern C++17 if-init syntax and follows the same pattern as
YSTDLIB_TRYbut without returning a value. The NOLINT directives are appropriate for suppressing warnings about macro usage since macros are necessary for this functionality.src/ystdlib/error_handling/test/test_Result.cpp (1)
15-29: LGTM! Well-structured test helpers.The test helpers are well-designed with clear semantics. The use of constexpr lambdas for
cVoidFuncandcIntFuncis a good modern C++ practice that enables compile-time evaluation when possible.
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3195 @Bill-hbrhbr I understand your preference for using ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ystdlib/error_handling/Result.hpp (1)
12-13: 🛠️ Refactor suggestionAdd documentation to explain the Result type alias
The Result type alias needs documentation explaining its purpose and why it's needed. This documentation would help users understand when and how to use this type in their code.
🧹 Nitpick comments (6)
src/ystdlib/error_handling/test/test_Result.cpp (6)
39-39: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE(BinaryErrorCode{BinaryErrorCodeEnum::Failure} == result_has_error.error()); + REQUIRE(BinaryErrorCode{BinaryErrorCodeEnum::Failure} == result_has_error.error());
53-53: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE((BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error())); + REQUIRE(BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error());
59-59: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE((cTestInt == result_no_error.value())); + REQUIRE(cTestInt == result_no_error.value());
77-77: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
84-84: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE((cTestInt == main_no_error.value())); + REQUIRE(cTestInt == main_no_error.value());
88-88: Remove unnecessary parentheses for consistencyThe equality expression doesn't need to be wrapped in parentheses here.
- REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/ystdlib/error_handling/Result.hpp(1 hunks)src/ystdlib/error_handling/test/test_Result.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/error_handling/Result.hppsrc/ystdlib/error_handling/test/test_Result.cpp
🧬 Code Definitions (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
src/ystdlib/error_handling/Result.hpp (1)
success(18-18)
🪛 Cppcheck (2.10-2)
src/ystdlib/error_handling/test/test_Result.cpp
[error] 42-42: syntax error
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (9)
src/ystdlib/error_handling/Result.hpp (3)
15-20: LGTM! Good documentationThe success() function is well-documented and its purpose is clear. The nodiscard attribute ensures that callers don't accidentally ignore successful results.
22-26: Consider using native Outcome implementationConsider using
https://ned14.github.io/outcome/reference/macros/tryx/for the underlying implementation. While the current implementation works, leveraging the native implementation might provide better maintainability and consistency with the library.
28-36: Consider renaming and using native implementationConsider:
- Changing the name to
YSTDLIB_TRYVto mirror Boost Outcome's native macro- Changing the underlying implementation to use
BOOST_OUTCOME_TRYVThis would improve consistency with the Outcome library's conventions and potentially provide better maintainability.
src/ystdlib/error_handling/test/test_Result.cpp (6)
15-29: LGTM! Well-structured test helper functionsThe test constants and helper functions are well-designed, providing clear testing scenarios for both success and error cases.
32-40: LGTM! Good test for ResultThis test case effectively verifies the behavior of Result for both success and error scenarios. The use of REQUIRE_FALSE is consistent with the project's preference for explicit testing.
42-54: LGTM! Effective test for error propagationThis test case properly verifies error propagation using the YSTDLIB_TRYV macro. The syntax error flagged by static analysis appears to be a false positive.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 42-42: syntax error
(syntaxError)
56-64: LGTM! Good test for ResultThis test case effectively verifies the behavior of Result for both success and error scenarios.
66-78: LGTM! Good test for propagating int errors to void resultsThis test case effectively verifies error propagation from a Result to a Result using the YSTDLIB_TRYV macro.
80-89: LGTM! Good test for direct error propagation with YSTDLIB_TRYXThis test case effectively verifies error propagation using the YSTDLIB_TRYX macro.
Bill-hbrhbr
changed the title
Result<T> alias and macros for Rust-like error handling.Result<T> alias and macros for Rust-like error handling.
Bill-hbrhbr
changed the title
Result<T> alias and macros for Rust-like error handling.
Bill-hbrhbr
changed the title
Result<T> alias and Rust-like error handling macros for standardized return-based error propagation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/ystdlib/error_handling/test/test_Result.cpp (3)
42-54: Good test for void result error propagation.The test effectively validates the YSTDLIB_ERROR_HANDLING_TRYV macro for propagating errors. Consider removing the unnecessary parentheses around the comparison on line 53 for consistency with other assertions.
- REQUIRE((BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error())); + REQUIRE(BinaryErrorCode{BinaryErrorCodeEnum::Failure} == main_has_error.error());🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 42-42: syntax error
(syntaxError)
66-78: Solid test for integer result propagation as void.This test effectively demonstrates how an integer result can be used in contexts where only error propagation matters (discarding the value). Consider removing the unnecessary parentheses on line 77 for consistency.
- REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
80-91: Great test for propagating integer results with values.This test case effectively demonstrates the YSTDLIB_ERROR_HANDLING_TRYX macro, showing how it correctly handles both returning the value and propagating errors. Consider removing the unnecessary parentheses on lines 86 and 90 for consistency.
- REQUIRE((cTestInt == main_no_error.value())); + REQUIRE(cTestInt == main_no_error.value()); - REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/ystdlib/error_handling/Result.hpp(1 hunks)src/ystdlib/error_handling/test/test_Result.cpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ystdlib/error_handling/Result.hpp
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/error_handling/test/test_Result.cpp
🧬 Code Definitions (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
src/ystdlib/error_handling/Result.hpp (1)
success(34-34)
🪛 Cppcheck (2.10-2)
src/ystdlib/error_handling/test/test_Result.cpp
[error] 42-42: syntax error
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: conventional-commits
- GitHub Check: build-ystdlib-cpp (macos-14)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (4)
src/ystdlib/error_handling/test/test_Result.cpp (4)
1-14: Imports and namespaces look good.The includes and namespace using declarations are well organized. Including the relevant headers for system errors, type traits, and the Result.hpp header being tested shows good practice.
15-29: Well-designed test helper functions.Good use of constexpr lambdas for the test helper functions. The implementation clearly demonstrates both success and error cases for void and int return types, providing a solid foundation for the test cases.
32-40: Thorough test case for basic Result functionality.This test case properly validates both the success and error scenarios for a void result. Good use of REQUIRE_FALSE for negated conditions, which aligns with the stated preference in the PR comments.
56-64: Effective test case for Result.The test properly validates both the success path (returning the integer value) and the error path. Good use of REQUIRE_FALSE for checking the absence of errors.
Co-authored-by: Lin Zhihao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/ystdlib/error_handling/test/test_Result.cpp (6)
68-68: Prefer direct comparison for improved readabilityThe parentheses around the comparison expression are unnecessary and inconsistent with other similar assertions in the file.
- REQUIRE((cTestInt == result_no_error.value())); + REQUIRE(cTestInt == result_no_error.value());
86-86: Unnecessary parentheses around comparisonSimilar to the previous case, these parentheses are unnecessary and inconsistent with style.
- REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
95-95: Redundant parentheses in test assertionFor consistency, remove the extra parentheses around the comparison expression.
- REQUIRE((cTestInt == main_no_error.value())); + REQUIRE(cTestInt == main_no_error.value());
99-99: Unnecessary parentheses in comparisonFor consistency, remove the extra parentheses around the comparison expression.
- REQUIRE((std::errc::bad_message == main_has_error.error())); + REQUIRE(std::errc::bad_message == main_has_error.error());
102-111: Comprehensive unique_ptr result testsThis test case effectively validates Result with move-only types, ensuring that the implementation correctly handles unique_ptr semantics. Consider removing unnecessary parentheses in the assertions for consistency.
- REQUIRE((cTestInt == *(result_no_error.value()))); + REQUIRE(cTestInt == *(result_no_error.value()));
127-138: Effective unique_ptr value propagation testsThis test case effectively validates error and value propagation for unique_ptr return types using the TRYX macro, ensuring move semantics are preserved. Consider removing unnecessary parentheses in the assertion for consistency.
- REQUIRE((cTestInt == *(main_no_error.value()))); + REQUIRE(cTestInt == *(main_no_error.value()));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/ystdlib/error_handling/Result.hpp(1 hunks)src/ystdlib/error_handling/test/test_Result.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/error_handling/Result.hppsrc/ystdlib/error_handling/test/test_Result.cpp
🧬 Code Definitions (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
src/ystdlib/error_handling/Result.hpp (1)
success(29-29)
🪛 Cppcheck (2.10-2)
src/ystdlib/error_handling/test/test_Result.cpp
[error] 51-51: syntax error
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (13)
src/ystdlib/error_handling/Result.hpp (7)
1-3: Well-structured header guardThe header guard follows standard naming conventions and correctly encapsulates the entire file.
4-10: Appropriate includesAll necessary headers for both standard library and the outcome library are included, providing a solid foundation for the error handling implementation.
11-24: Well-documented Result type aliasThe Result type alias and its documentation are clear and comprehensive. The alias effectively standardizes error handling by defaulting to std::error_code, facilitating interoperability with ErrorCode types.
25-31: Utility function for void resultsThe success() function enhances readability for functions returning Result, providing a clear way to indicate successful completion.
33-49: Rust-like error propagation macro with proper compiler guardsThe TRYX macro effectively emulates Rust's ? operator for error propagation, with clear documentation about its behavior and compiler requirements. The implementation correctly uses the underlying OUTCOME_TRYX and includes appropriate compiler checks.
51-63: Well-implemented TRYV macro for void resultsThe TRYV macro provides a clean way to propagate errors from Result expressions, with comprehensive documentation explaining its behavior and use cases.
64-66: Properly closed namespace and header guardThe namespace and header guard are correctly closed at the end of the file.
src/ystdlib/error_handling/test/test_Result.cpp (6)
1-10: Appropriate includes and dependenciesAll necessary headers are included, following a logical order from standard libraries to project-specific ones, providing a solid foundation for the test cases.
11-17: Clear using declarationsUsing declarations improve readability by avoiding namespace qualifications in the test code, making the tests more concise and focused.
18-38: Well-implemented test helpersThe test helpers provide a robust foundation for testing various Result scenarios with different return types (void, int, unique_ptr). The unique_ptr test case is particularly valuable as it verifies that the macro correctly handles move-only types.
40-49: Comprehensive void result testsThe test case thoroughly validates Result behavior for both success and error conditions, correctly using REQUIRE_FALSE as per project preferences.
51-63: Thorough void propagation testsThis test case effectively verifies error propagation in Result contexts using the TRYV macro, covering both success and error scenarios.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 51-51: syntax error
(syntaxError)
113-125: Thorough unique_ptr error propagation testsThe test case comprehensively validates error propagation with move-only types using the TRYV macro, covering both success and error scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
32-37: 🛠️ Refactor suggestionConsider adding a test that explicitly verifies move semantics.
This function properly returns a unique pointer, but there's no explicit test to verify that move semantics are used rather than copy semantics (as the unique_ptr copy constructor is deleted).
Add a test that explicitly confirms the move semantics:
void verify_unique_ptr_is_moved() { auto ptr = cUniquePtrFunc(false).value(); REQUIRE(cTestInt == *ptr); // ptr should be valid and the original unique_ptr in the Result should be moved-from }
🧹 Nitpick comments (3)
src/ystdlib/error_handling/test/test_Result.cpp (3)
18-38: Lambda function names could be more descriptive.The test helpers are well-implemented, but consider using more descriptive names than
cVoidFunc,cIntFunc, andcUniquePtrFuncto better communicate their purpose (e.g.,getVoidResult,getIntResult,getUniquePtrResult).Additionally, in
cUniquePtrFunc, the error case returnsAlwaysSuccessErrorCode{AlwaysSuccessErrorCodeEnum::Success}which is counterintuitive since this is the error path. Consider using an error code that better represents failure.
102-111: Good test for unique pointer results, but could verify move semantics.While this test properly verifies basic functionality with unique pointers, it doesn't explicitly test that the unique pointer is moved rather than copied (which would be impossible as the copy constructor is deleted).
1-139: Consider adding tests for additional edge cases.The current tests cover basic functionality well, but consider adding tests for:
- Default construction of
Result- Move/copy semantics for copyable types
- Error conversion between different error types
- Chaining multiple
YSTDLIB_ERROR_HANDLING_TRYXorYSTDLIB_ERROR_HANDLING_TRYVcalls🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 51-51: syntax error
(syntaxError)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ystdlib/error_handling/test/test_Result.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/error_handling/test/test_Result.cpp
🧬 Code Definitions (1)
src/ystdlib/error_handling/test/test_Result.cpp (1)
src/ystdlib/error_handling/Result.hpp (1)
success(29-29)
🪛 Cppcheck (2.10-2)
src/ystdlib/error_handling/test/test_Result.cpp
[error] 51-51: syntax error
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (8)
src/ystdlib/error_handling/test/test_Result.cpp (8)
1-17: Well-organized imports and using statements.The includes cover all necessary headers for the test suite and the using statements clearly identify all the types from the
ystdlib::error_handlingnamespace that are used throughout the tests.
43-44: REQUIRE_FALSE usage is good despite coding guidelines.While the coding guidelines recommend
false == <expression>rather than!<expression>, the use ofREQUIRE_FALSEhere is appropriate for testing as it provides clearer failure messages.
51-63: Good test for error propagation with void results.This test properly verifies that the
YSTDLIB_ERROR_HANDLING_TRYVmacro correctly propagates errors from a void result function.🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 51-51: syntax error
(syntaxError)
65-73: Good test for basic int result functionality.This test properly checks both success and error cases for
Result<int>.
75-87: Good test for error propagation from int to void results.This test properly verifies that the
YSTDLIB_ERROR_HANDLING_TRYVmacro correctly propagates errors from an int result function to a void result function.
89-100: Good test for error propagation with int results.This test properly verifies that the
YSTDLIB_ERROR_HANDLING_TRYXmacro correctly handles both success and error cases for int results.
113-125: Good test for error propagation from unique pointer to void results.This test properly verifies that the
YSTDLIB_ERROR_HANDLING_TRYVmacro correctly propagates errors from a unique pointer result function to a void result function.
127-138: Good test for error propagation with unique pointer results.This test properly verifies that the
YSTDLIB_ERROR_HANDLING_TRYXmacro correctly handles both success and error cases for unique pointer results.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/ystdlib/error_handling/CMakeLists.txt (1)
7-7: Adding theResult.hpppublic header is appropriate.
This new header aligns with the PR's goal of standardising error handling. Please ensure that all intended public symbols and documentation withinResult.hppare verified and consistent with the library's existing conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ystdlib/error_handling/CMakeLists.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (2)
src/ystdlib/error_handling/CMakeLists.txt (2)
10-10: Inclusion ofoutcome::hlin PUBLIC_LINK_LIBRARIES.
Adding this dependency is consistent with extending the library’s error handling capabilities. It is advisable to confirm that the specified version ofoutcome::hlis compatible with the rest of the codebase and properly version‐locked if necessary.
14-14: Adding the new test sourcetest/test_Result.cppenhances coverage.
This addition is beneficial for validating the newResultand error handling macros. Please ensure that the test cases within this file cover both successful and error conditions, and consider adhering to our preference for usingREQUIRE_FALSEwhere applicable for clarity.
Bill-hbrhbr
changed the title
Result<T> alias and Rust-like error handling macros for standardized return-based error propagation.Result<T, E> alias and macros to enable ergonomic Rust-style error handling.
Description
This PR builds upon #39 by introducing macros and type aliases to simplify handling functions that return
outcome::std_result<ReturnType, ErrorType>.Introduces a type alias
Result<ReturnType>that provides concise function return types likeResult<int>orResult<void>. TheErrorTypedefaults tostd::error_code, leveraging the fact that allystdlib::error_handling::ErrorCodetypes can be implicitly converted tostd::error_code.Introduces two utility macros, in styles inspired by Rust and Outcome:
YSTDLIB_ERROR_HANDLING_TRYX(expr): Propagates either a value or an error from aResult<T>expression, mimicking Rust’s?operator. On success, the value is returned from the expression; on failure, the error is propagated to the caller.YSTDLIB_ERROR_HANDLING_TRYV(expr): Similar toYSTDLIB_ERROR_HANDLING_TRYX, but forResult<void>. It checks for an error and propagates it if present, without returning a value.By standardizing on
std::error_codeas our defaultErrorType, these macros also allow us to propagate error codes across different error code systems easily.Finally, we add
ystdlib::error_handling::success()as a helper for returning success inResult<void>functions, improving readability and making success returns more explicit than a bare return.Checklist
breaking change.
Validation performed
test_Result.hppand passed all its unit tests.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests