Skip to content

Commit 1488492

Browse files
elbenoelbeno
committed
🎨 Improve flow error messages
Problem: - Flow errors use a hacky technique to output useful information. Solution: - Use `STATIC_ASSERT`.
1 parent 0842a1d commit 1488492

File tree

6 files changed

+63
-105
lines changed

6 files changed

+63
-105
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ include(cmake/string_catalog.cmake)
2525
add_versioned_package("gh:boostorg/mp11#boost-1.83.0")
2626
fmt_recipe(10.2.1)
2727
add_versioned_package("gh:intel/cpp-baremetal-concurrency#7c5b26c")
28-
add_versioned_package("gh:intel/cpp-std-extensions#5530b5d")
28+
add_versioned_package("gh:intel/cpp-std-extensions#73569fe")
2929
add_versioned_package("gh:intel/cpp-baremetal-senders-and-receivers#73d95bc")
3030

3131
add_library(cib INTERFACE)

‎include/flow/graph_builder.hpp‎

Lines changed: 41 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
#include <flow/detail/walk.hpp>
55
#include <flow/impl.hpp>
66

7-
#include <stdx/ct_format.hpp>
87
#include <stdx/ct_string.hpp>
98
#include <stdx/cx_multimap.hpp>
109
#include <stdx/cx_set.hpp>
1110
#include <stdx/cx_vector.hpp>
1211
#include <stdx/span.hpp>
12+
#include <stdx/static_assert.hpp>
1313
#include <stdx/tuple_algorithms.hpp>
1414
#include <stdx/type_traits.hpp>
1515
#include <stdx/utility.hpp>
@@ -52,31 +52,10 @@ template <typename T> using name_for = typename T::name_t;
5252
});
5353
}
5454

55-
template <typename Cond> struct sequence_conditions;
56-
57-
template <auto...> struct INFO_flow_name_;
58-
59-
template <auto...> struct INFO_step_a_name_;
60-
61-
template <auto...> struct INFO_step_a_condition_;
62-
63-
template <auto...> struct INFO_step_b_name_;
64-
65-
template <auto...> struct INFO_step_b_condition_;
66-
67-
template <auto...> struct INFO_sequence_condition_;
68-
69-
template <typename...> struct INFO_sequence_missing_predicate_;
70-
71-
template <typename...> constexpr bool ERROR_DETAILS_ = false;
72-
73-
template <auto...> struct INFO_missing_steps_;
74-
75-
template <auto...> struct INFO_duplicated_steps_;
76-
7755
template <stdx::ct_string Name,
7856
template <stdx::ct_string, std::size_t> typename Impl>
7957
struct graph_builder {
58+
// NOLINTBEGIN(readability-function-cognitive-complexity)
8059
template <typename Node, std::size_t N, std::size_t E>
8160
[[nodiscard]] constexpr static auto make_graph(auto const &nodes,
8261
auto const &edges) {
@@ -100,31 +79,17 @@ struct graph_builder {
10079

10180
stdx::for_each(
10281
[&]<typename P>(P const &) {
103-
if constexpr (not stdx::contains_type<edge_ps_t, P>) {
104-
static_assert(
105-
ERROR_DETAILS_<
106-
INFO_flow_name_<Name>,
107-
INFO_step_a_name_<lhs_t::ct_name>,
108-
INFO_step_a_condition_<
109-
lhs_t::condition.ct_name>,
110-
INFO_step_b_name_<rhs_t::ct_name>,
111-
INFO_step_b_condition_<
112-
rhs_t::condition.ct_name>,
113-
INFO_sequence_condition_<Cond::ct_name>,
114-
INFO_sequence_missing_predicate_<P>>,
115-
116-
"The conditions on this sequence "
117-
"(step_a >> step_b) are weaker than those on "
118-
"step_a and/or step_b. The sequence could be "
119-
"enabled while step_a and/or step_b is not. "
120-
"Specifically, the sequence is missing the "
121-
"predicate identified in "
122-
"`INFO_sequence_missing_predicate_`. TIP: "
123-
"Look for 'ERROR_DETAILS_` and `INFO_` in "
124-
"the compiler error message for details on "
125-
"the sequence, the step names, and the "
126-
"conditions.");
127-
}
82+
STATIC_ASSERT(
83+
(stdx::contains_type<edge_ps_t, P>),
84+
"The conditions on the sequence ({} >> {})[{}] are "
85+
"weaker than those on {}[{}] or {}[{}]. "
86+
"Specifically, the sequence is missing the "
87+
"predicate: {}",
88+
CX_VALUE(lhs_t::ct_name), CX_VALUE(rhs_t::ct_name),
89+
CX_VALUE(Cond::ct_name), CX_VALUE(lhs_t::ct_name),
90+
CX_VALUE(lhs_t::condition.ct_name),
91+
CX_VALUE(rhs_t::ct_name),
92+
CX_VALUE(rhs_t::condition.ct_name), CX_VALUE(P));
12893
},
12994
node_ps);
13095

@@ -133,6 +98,7 @@ struct graph_builder {
13398
edges);
13499
return g;
135100
}
101+
// NOLINTEND(readability-function-cognitive-complexity)
136102

137103
template <typename Node, typename Graph>
138104
[[nodiscard]] constexpr static auto is_source_of(Node const &node,
@@ -191,6 +157,15 @@ struct graph_builder {
191157
return std::optional<Output>{std::in_place, span_t{ordered_list}};
192158
}
193159

160+
template <typename T>
161+
constexpr static auto error_steps =
162+
stdx::transform([](auto N) { return stdx::ct_string_from_type(N); },
163+
T{})
164+
.join(stdx::ct_string{""}, [](auto x, auto y) {
165+
using namespace stdx::literals;
166+
return x + ", "_cts + y;
167+
});
168+
194169
constexpr static void check_for_missing_nodes(auto nodes,
195170
auto mentioned_nodes) {
196171
constexpr auto get_name = []<typename N>(N) ->
@@ -201,52 +176,24 @@ struct graph_builder {
201176
using node_names_t = decltype(stdx::to_sorted_set(node_names));
202177
using mentioned_node_names_t =
203178
decltype(stdx::to_sorted_set(mentioned_node_names));
204-
205-
if constexpr (not std::is_same_v<node_names_t,
206-
mentioned_node_names_t>) {
207-
using missing_nodes_t =
208-
boost::mp11::mp_set_difference<mentioned_node_names_t,
209-
node_names_t>;
210-
211-
constexpr auto missing_nodes = missing_nodes_t{};
212-
constexpr auto missing_nodes_ct_strings = stdx::transform(
213-
[]<typename N>(N) { return stdx::ct_string_from_type(N{}); },
214-
missing_nodes);
215-
216-
[&]<std::size_t... Is>(std::index_sequence<Is...>) {
217-
using error_details_t = INFO_missing_steps_<stdx::get<Is>(
218-
missing_nodes_ct_strings)...>;
219-
220-
static_assert(
221-
ERROR_DETAILS_<INFO_flow_name_<Name>, error_details_t>,
222-
"One or more steps are referenced in the flow but not "
223-
"explicitly added with the * operator. The "
224-
"beginning of this error shows you which steps are "
225-
"missing.");
226-
}(std::make_index_sequence<
227-
stdx::tuple_size_v<decltype(missing_nodes)>>{});
228-
}
229-
230-
if constexpr (stdx::tuple_size_v<node_names_t> !=
231-
stdx::tuple_size_v<decltype(node_names)>) {
232-
constexpr auto duplicates = stdx::transform(
233-
[](auto e) {
234-
return stdx::ct_string_from_type(stdx::get<0>(e));
235-
},
236-
stdx::filter<detail::is_duplicated>(stdx::gather(node_names)));
237-
238-
[&]<std::size_t... Is>(std::index_sequence<Is...>) {
239-
using error_details_t =
240-
INFO_duplicated_steps_<stdx::get<Is>(duplicates)...>;
241-
242-
static_assert(
243-
ERROR_DETAILS_<INFO_flow_name_<Name>, error_details_t>,
244-
"One or more steps are explicitly added more than once "
245-
"using the * operator. The beginning of this "
246-
"error shows you which steps are duplicated.");
247-
}(std::make_index_sequence<
248-
stdx::tuple_size_v<decltype(duplicates)>>{});
249-
}
179+
using missing_nodes_t =
180+
boost::mp11::mp_set_difference<mentioned_node_names_t,
181+
node_names_t>;
182+
STATIC_ASSERT(
183+
(std::is_same_v<node_names_t, mentioned_node_names_t>),
184+
"One or more steps are referenced in the flow ({}) but not "
185+
"explicitly added with the * operator. The missing steps are: {}.",
186+
CX_VALUE(Name), CX_VALUE(error_steps<missing_nodes_t>));
187+
188+
constexpr auto duplicates = stdx::transform(
189+
[](auto e) { return stdx::get<0>(e); },
190+
stdx::filter<detail::is_duplicated>(stdx::gather(node_names)));
191+
using duplicate_nodes_t = decltype(duplicates);
192+
STATIC_ASSERT(
193+
stdx::tuple_size_v<duplicate_nodes_t> == 0,
194+
"One or more steps in the flow ({}) are explicitly added more than "
195+
"once using the * operator. The duplicate steps are: {}.",
196+
CX_VALUE(Name), CX_VALUE(error_steps<duplicate_nodes_t>));
250197
}
251198

252199
template <typename Graph>

‎test/flow/fail/CMakeLists.txt‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
1-
add_compile_fail_test(cyclic_flow.cpp LIBRARIES warnings cib)
2-
add_compile_fail_test(only_reference_added.cpp LIBRARIES warnings cib)
3-
add_compile_fail_test(mismatched_flow_runtime_conditional.cpp LIBRARIES
4-
warnings cib)
5-
add_compile_fail_test(node_explicitly_added_twice.cpp LIBRARIES warnings cib)
1+
add_compile_fail_test(cyclic_flow.cpp LIBRARIES cib)
2+
3+
function(add_formatted_errors_tests)
4+
add_compile_fail_test(only_reference_added.cpp LIBRARIES cib)
5+
add_compile_fail_test(mismatched_flow_runtime_conditional.cpp LIBRARIES cib)
6+
add_compile_fail_test(node_explicitly_added_twice.cpp LIBRARIES cib)
7+
endfunction()
8+
9+
if(${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang" AND ${CMAKE_CXX_COMPILER_VERSION}
10+
VERSION_GREATER_EQUAL 15)
11+
add_formatted_errors_tests()
12+
endif()
13+
if(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU" AND ${CMAKE_CXX_COMPILER_VERSION}
14+
VERSION_GREATER_EQUAL 13.2)
15+
add_formatted_errors_tests()
16+
endif()

‎test/flow/fail/mismatched_flow_runtime_conditional.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include <cib/cib.hpp>
22
#include <flow/flow.hpp>
33

4-
// EXPECT: The conditions on this sequence
4+
// EXPECT: The conditions on the sequence
55

66
namespace {
77
using namespace flow::literals;

‎test/flow/fail/node_explicitly_added_twice.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
#include <cib/cib.hpp>
22
#include <flow/flow.hpp>
33

4-
// EXPECT: One or more steps are explicitly added more than once
4+
// EXPECT: are explicitly added more than once
55

66
namespace {
77
using namespace flow::literals;
88

99
constexpr auto a = flow::milestone<"a">();
1010

11-
struct TestFlowAlpha : public flow::service<> {};
11+
struct TestFlowAlpha : public flow::service<"alpha"> {};
1212

1313
struct FlowConfig {
1414
constexpr static auto config = cib::config(

‎test/flow/fail/only_reference_added.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
#include <cib/cib.hpp>
22
#include <flow/flow.hpp>
33

4-
// EXPECT: One or more steps are referenced in the flow but not explicitly
4+
// EXPECT: One or more steps are referenced in the flow
55

66
namespace {
77
using namespace flow::literals;
88

99
constexpr auto a = flow::milestone<"a">();
1010

11-
struct TestFlowAlpha : public flow::service<> {};
11+
struct TestFlowAlpha : public flow::service<"alpha"> {};
1212

1313
struct FlowConfig {
1414
constexpr static auto config =

0 commit comments

Comments
 (0)