Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🎨 Improve flow error messages #637

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🎨 Improve flow error messages #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 46 additions & 76 deletions include/flow/graph_builder.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,31 +52,10 @@ template <typename T> using name_for = typename T::name_t;
});
}

template <typename Cond> struct sequence_conditions;

template <auto...> struct INFO_flow_name_;

template <auto...> struct INFO_step_a_name_;

template <auto...> struct INFO_step_a_condition_;

template <auto...> struct INFO_step_b_name_;

template <auto...> struct INFO_step_b_condition_;

template <auto...> struct INFO_sequence_condition_;

template <typename...> struct INFO_sequence_missing_predicate_;

template <typename...> constexpr bool ERROR_DETAILS_ = false;

template <auto...> struct INFO_missing_steps_;

template <auto...> struct INFO_duplicated_steps_;

template <stdx::ct_string Name,
template <stdx::ct_string, std::size_t> typename Impl>
struct graph_builder {
// NOLINTBEGIN(readability-function-cognitive-complexity)
template <typename Node, std::size_t N, std::size_t E>
[[nodiscard]] constexpr static auto make_graph(auto const &nodes,
auto const &edges) {
Expand All @@ -101,29 +80,20 @@ struct graph_builder {
stdx::for_each(
[&]<typename P>(P const &) {
if constexpr (not stdx::contains_type<edge_ps_t, P>) {
static_assert(
ERROR_DETAILS_<
INFO_flow_name_<Name>,
INFO_step_a_name_<lhs_t::ct_name>,
INFO_step_a_condition_<
lhs_t::condition.ct_name>,
INFO_step_b_name_<rhs_t::ct_name>,
INFO_step_b_condition_<
rhs_t::condition.ct_name>,
INFO_sequence_condition_<Cond::ct_name>,
INFO_sequence_missing_predicate_<P>>,

"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<false>.template emit<error>();
}
},
node_ps);
Expand All @@ -133,6 +103,7 @@ struct graph_builder {
edges);
return g;
}
// NOLINTEND(readability-function-cognitive-complexity)

template <typename Node, typename Graph>
[[nodiscard]] constexpr static auto is_source_of(Node const &node,
Expand Down Expand Up @@ -208,44 +179,43 @@ struct graph_builder {
boost::mp11::mp_set_difference<mentioned_node_names_t,
node_names_t>;

constexpr auto missing_nodes = missing_nodes_t{};
constexpr auto missing_nodes_ct_strings = stdx::transform(
[]<typename N>(N) { return stdx::ct_string_from_type(N{}); },
missing_nodes);

[&]<std::size_t... Is>(std::index_sequence<Is...>) {
using error_details_t = INFO_missing_steps_<stdx::get<Is>(
missing_nodes_ct_strings)...>;

static_assert(
ERROR_DETAILS_<INFO_flow_name_<Name>, 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<decltype(missing_nodes)>>{});
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(
[]<typename N>(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<false>.template emit<error>();
}

if constexpr (stdx::tuple_size_v<node_names_t> !=
stdx::tuple_size_v<decltype(node_names)>) {
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<detail::is_duplicated>(stdx::gather(node_names)));

[&]<std::size_t... Is>(std::index_sequence<Is...>) {
using error_details_t =
INFO_duplicated_steps_<stdx::get<Is>(duplicates)...>;

static_assert(
ERROR_DETAILS_<INFO_flow_name_<Name>, 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<decltype(duplicates)>>{});
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(
[]<typename N>(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<false>.template emit<error>();
}
}

Expand Down
21 changes: 16 additions & 5 deletions test/flow/fail/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
2 changes: 1 addition & 1 deletion test/flow/fail/mismatched_flow_runtime_conditional.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <cib/cib.hpp>
#include <flow/flow.hpp>

// EXPECT: The conditions on this sequence
// EXPECT: The conditions on the sequence

namespace {
using namespace flow::literals;
Expand Down
4 changes: 2 additions & 2 deletions test/flow/fail/node_explicitly_added_twice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#include <cib/cib.hpp>
#include <flow/flow.hpp>

// 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(
Expand Down
4 changes: 2 additions & 2 deletions test/flow/fail/only_reference_added.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#include <cib/cib.hpp>
#include <flow/flow.hpp>

// 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 =
Expand Down