From 27d1f0be57b65216316be47fa3fb82a320c3feac Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Thu, 24 Oct 2024 16:36:33 -0600 Subject: [PATCH] :art: Improve flow error messages Problem: - Flow errors use a hacky technique to output useful information. Solution: - Use `stdx::ct_check`. --- include/flow/graph_builder.hpp | 122 +++++++----------- test/flow/fail/CMakeLists.txt | 21 ++- .../mismatched_flow_runtime_conditional.cpp | 2 +- .../flow/fail/node_explicitly_added_twice.cpp | 4 +- test/flow/fail/only_reference_added.cpp | 4 +- 5 files changed, 67 insertions(+), 86 deletions(-) diff --git a/include/flow/graph_builder.hpp b/include/flow/graph_builder.hpp index dd470d68..390fb540 100644 --- a/include/flow/graph_builder.hpp +++ b/include/flow/graph_builder.hpp @@ -52,31 +52,10 @@ template using name_for = typename T::name_t; }); } -template struct sequence_conditions; - -template struct INFO_flow_name_; - -template struct INFO_step_a_name_; - -template struct INFO_step_a_condition_; - -template struct INFO_step_b_name_; - -template struct INFO_step_b_condition_; - -template struct INFO_sequence_condition_; - -template struct INFO_sequence_missing_predicate_; - -template constexpr bool ERROR_DETAILS_ = false; - -template struct INFO_missing_steps_; - -template struct INFO_duplicated_steps_; - template typename Impl> struct graph_builder { + // NOLINTBEGIN(readability-function-cognitive-complexity) template [[nodiscard]] constexpr static auto make_graph(auto const &nodes, auto const &edges) { @@ -101,29 +80,20 @@ struct graph_builder { stdx::for_each( [&](P const &) { if constexpr (not stdx::contains_type) { - static_assert( - ERROR_DETAILS_< - INFO_flow_name_, - INFO_step_a_name_, - INFO_step_a_condition_< - lhs_t::condition.ct_name>, - INFO_step_b_name_, - INFO_step_b_condition_< - rhs_t::condition.ct_name>, - INFO_sequence_condition_, - INFO_sequence_missing_predicate_

>, - - "The conditions on this sequence " - "(step_a >> step_b) are weaker than those on " - "step_a and/or step_b. The sequence could be " - "enabled while step_a and/or step_b is not. " - "Specifically, the sequence is missing the " - "predicate identified in " - "`INFO_sequence_missing_predicate_`. TIP: " - "Look for 'ERROR_DETAILS_` and `INFO_` in " - "the compiler error message for details on " - "the sequence, the step names, and the " - "conditions."); + constexpr auto error = stdx::ct_format< + "The conditions on the sequence ({} >> " + "{})[{}] are weaker than those on {}[{}] or " + "{}[{}]. Specifically, the sequence is missing " + "the predicate: {}">( + CX_VALUE(lhs_t::ct_name), + CX_VALUE(rhs_t::ct_name), + CX_VALUE(Cond::ct_name), + CX_VALUE(lhs_t::ct_name), + CX_VALUE(lhs_t::condition.ct_name), + CX_VALUE(rhs_t::ct_name), + CX_VALUE(rhs_t::condition.ct_name), + CX_VALUE(P)); + stdx::ct_check.template emit(); } }, node_ps); @@ -133,6 +103,7 @@ struct graph_builder { edges); return g; } + // NOLINTEND(readability-function-cognitive-complexity) template [[nodiscard]] constexpr static auto is_source_of(Node const &node, @@ -208,44 +179,43 @@ struct graph_builder { boost::mp11::mp_set_difference; - constexpr auto missing_nodes = missing_nodes_t{}; - constexpr auto missing_nodes_ct_strings = stdx::transform( - [](N) { return stdx::ct_string_from_type(N{}); }, - missing_nodes); - - [&](std::index_sequence) { - using error_details_t = INFO_missing_steps_( - missing_nodes_ct_strings)...>; - - static_assert( - ERROR_DETAILS_, error_details_t>, - "One or more steps are referenced in the flow but not " - "explicitly added with the * operator. The " - "beginning of this error shows you which steps are " - "missing."); - }(std::make_index_sequence< - stdx::tuple_size_v>{}); + constexpr auto error = + stdx::ct_format<"One or more steps are referenced in the " + "flow ({}) but not explicitly added with the " + "* operator. The missing steps are: {}.">( + CX_VALUE(Name), + CX_VALUE(stdx::transform( + [](N) { + return stdx::ct_string_from_type(N{}); + }, + missing_nodes_t{}) + .join([](auto x, auto y) { + using namespace stdx::literals; + return x + ", "_cts + y; + }))); + stdx::ct_check.template emit(); } if constexpr (stdx::tuple_size_v != stdx::tuple_size_v) { constexpr auto duplicates = stdx::transform( - [](auto e) { - return stdx::ct_string_from_type(stdx::get<0>(e)); - }, + [](auto e) { return stdx::get<0>(e); }, stdx::filter(stdx::gather(node_names))); - [&](std::index_sequence) { - using error_details_t = - INFO_duplicated_steps_(duplicates)...>; - - static_assert( - ERROR_DETAILS_, error_details_t>, - "One or more steps are explicitly added more than once " - "using the * operator. The beginning of this " - "error shows you which steps are duplicated."); - }(std::make_index_sequence< - stdx::tuple_size_v>{}); + constexpr auto error = stdx::ct_format< + "One or more steps in the flow ({}) are explicitly added more " + "than once using the * operator. The duplicate steps are: {}.">( + CX_VALUE(Name), + CX_VALUE(stdx::transform( + [](N) { + return stdx::ct_string_from_type(N{}); + }, + decltype(duplicates){}) + .join([](auto x, auto y) { + using namespace stdx::literals; + return x + ", "_cts + y; + }))); + stdx::ct_check.template emit(); } } diff --git a/test/flow/fail/CMakeLists.txt b/test/flow/fail/CMakeLists.txt index 7c41db00..d33bcb49 100644 --- a/test/flow/fail/CMakeLists.txt +++ b/test/flow/fail/CMakeLists.txt @@ -1,5 +1,16 @@ -add_compile_fail_test(cyclic_flow.cpp LIBRARIES warnings cib) -add_compile_fail_test(only_reference_added.cpp LIBRARIES warnings cib) -add_compile_fail_test(mismatched_flow_runtime_conditional.cpp LIBRARIES - warnings cib) -add_compile_fail_test(node_explicitly_added_twice.cpp LIBRARIES warnings cib) +add_compile_fail_test(cyclic_flow.cpp LIBRARIES cib) + +function(add_formatted_errors_tests) + add_compile_fail_test(only_reference_added.cpp LIBRARIES cib) + add_compile_fail_test(mismatched_flow_runtime_conditional.cpp LIBRARIES cib) + add_compile_fail_test(node_explicitly_added_twice.cpp LIBRARIES cib) +endfunction() + +if(${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang" AND ${CMAKE_CXX_COMPILER_VERSION} + VERSION_GREATER_EQUAL 15) + add_formatted_errors_tests() +endif() +if(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU" AND ${CMAKE_CXX_COMPILER_VERSION} + VERSION_GREATER_EQUAL 13.2) + add_formatted_errors_tests() +endif() diff --git a/test/flow/fail/mismatched_flow_runtime_conditional.cpp b/test/flow/fail/mismatched_flow_runtime_conditional.cpp index 32c16854..999f7116 100644 --- a/test/flow/fail/mismatched_flow_runtime_conditional.cpp +++ b/test/flow/fail/mismatched_flow_runtime_conditional.cpp @@ -1,7 +1,7 @@ #include #include -// EXPECT: The conditions on this sequence +// EXPECT: The conditions on the sequence namespace { using namespace flow::literals; diff --git a/test/flow/fail/node_explicitly_added_twice.cpp b/test/flow/fail/node_explicitly_added_twice.cpp index ba99bb4f..1d89e508 100644 --- a/test/flow/fail/node_explicitly_added_twice.cpp +++ b/test/flow/fail/node_explicitly_added_twice.cpp @@ -1,14 +1,14 @@ #include #include -// EXPECT: One or more steps are explicitly added more than once +// EXPECT: are explicitly added more than once namespace { using namespace flow::literals; constexpr auto a = flow::milestone<"a">(); -struct TestFlowAlpha : public flow::service<> {}; +struct TestFlowAlpha : public flow::service<"alpha"> {}; struct FlowConfig { constexpr static auto config = cib::config( diff --git a/test/flow/fail/only_reference_added.cpp b/test/flow/fail/only_reference_added.cpp index 44130097..cd9f4c48 100644 --- a/test/flow/fail/only_reference_added.cpp +++ b/test/flow/fail/only_reference_added.cpp @@ -1,14 +1,14 @@ #include #include -// EXPECT: One or more steps are referenced in the flow but not explicitly +// EXPECT: One or more steps are referenced in the flow namespace { using namespace flow::literals; constexpr auto a = flow::milestone<"a">(); -struct TestFlowAlpha : public flow::service<> {}; +struct TestFlowAlpha : public flow::service<"alpha"> {}; struct FlowConfig { constexpr static auto config =