Skip to content

Commit

Permalink
Style cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdemoret-nv
mdemoret-nv committed Oct 22, 2024
1 parent 520bf8f commit 8554c66
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 76 deletions.
2 changes: 1 addition & 1 deletion cpp/mrc/include/mrc/node/operators/combine_latest.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CombineLatestBase<std::tuple<InputT...>, OutputT>
m_upstream_holders(build_ingress(const_cast<CombineLatestBase*>(this), std::index_sequence_for<InputT...>{}))
{}

virtual ~CombineLatestBase() = default;
~CombineLatestBase() override = default;

template <size_t N>
std::shared_ptr<edge::IWritableProvider<NthTypeOf<N, InputT...>>> get_sink() const
Expand Down
2 changes: 1 addition & 1 deletion cpp/mrc/include/mrc/node/operators/with_latest_from.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class WithLatestFromBase<std::tuple<InputT...>, OutputT>
m_upstream_holders(build_ingress(const_cast<WithLatestFromBase*>(this), std::index_sequence_for<InputT...>{}))
{}

virtual ~WithLatestFromBase() = default;
~WithLatestFromBase() override = default;

template <size_t N>
std::shared_ptr<edge::IWritableProvider<NthTypeOf<N, InputT...>>> get_sink() const
Expand Down
2 changes: 1 addition & 1 deletion cpp/mrc/include/mrc/node/operators/zip.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class ZipBase<std::tuple<InputT...>, OutputT> : public ZipTypelessBase,
m_queue_counts.fill(0);
}

virtual ~ZipBase() = default;
~ZipBase() override = default;

template <size_t N>
std::shared_ptr<edge::IWritableProvider<NthTypeOf<N, InputT...>>> get_sink() const
Expand Down
6 changes: 3 additions & 3 deletions cpp/mrc/include/mrc/segment/builder.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,9 @@ void IBuilder::make_edge(SourceObjectT source, SinkObjectT sink)
sink_sp_type_t, // Fallback to Sink deduced type
SinkNodeTypeT>; // Fallback to Sink explicit hint
using deduced_sink_type_t = first_non_void_type_t<sink_sp_type_t, // Deduced type (if possible)
SinkNodeTypeT, // Explicit type hint
source_sp_type_t, // Fallback to Source deduced type
SourceNodeTypeT>; // Fallback to Source explicit hint
SinkNodeTypeT, // Explicit type hint
source_sp_type_t, // Fallback to Source deduced type
SourceNodeTypeT>; // Fallback to Source explicit hint

VLOG(2) << "Deduced source type: " << mrc::type_name<deduced_source_type_t>() << std::endl;
VLOG(2) << "Deduced sink type: " << mrc::type_name<deduced_sink_type_t>() << std::endl;
Expand Down
26 changes: 13 additions & 13 deletions cpp/mrc/include/mrc/segment/object.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ struct ObjectPropertiesState
// Will be set by the builder class when the object is added to a segment
bool m_is_initialized{false};

std::string m_name{""};
std::string m_name;

// The owning builder. Once set, name cannot be changed
IBuilder* m_owning_builder{nullptr};
Expand Down Expand Up @@ -241,9 +241,9 @@ edge::IWritableAcceptor<T>& ObjectProperties::writable_acceptor_typed()

if (writable_acceptor == nullptr)
{
LOG(ERROR) << "Failed to cast " << type_name() << " to " << "IWritableAcceptor<"
<< std::string(mrc::type_name<T>()) << ">" << "IWritableAcceptor<"
<< ::mrc::type_name(base.writable_acceptor_type().full_type()) << ">.";
LOG(ERROR) << "Failed to cast " << type_name() << " to "
<< "IWritableAcceptor<" << std::string(mrc::type_name<T>()) << ">"
<< "IWritableAcceptor<" << ::mrc::type_name(base.writable_acceptor_type().full_type()) << ">.";
throw exceptions::MrcRuntimeError("Failed to cast Sink to requested IWritableAcceptor<T>");
}

Expand All @@ -258,9 +258,9 @@ edge::IWritableProvider<T>& ObjectProperties::writable_provider_typed()

if (writable_provider == nullptr)
{
LOG(ERROR) << "Failed to cast " << type_name() << " to " << "IWritableProvider<"
<< std::string(mrc::type_name<T>()) << ">" << "IWritableProvider<"
<< ::mrc::type_name(base.writable_provider_type().full_type()) << ">.";
LOG(ERROR) << "Failed to cast " << type_name() << " to "
<< "IWritableProvider<" << std::string(mrc::type_name<T>()) << ">"
<< "IWritableProvider<" << ::mrc::type_name(base.writable_provider_type().full_type()) << ">.";
throw exceptions::MrcRuntimeError("Failed to cast Sink to requested IWritableProvider<T>");
}

Expand All @@ -275,9 +275,9 @@ edge::IReadableAcceptor<T>& ObjectProperties::readable_acceptor_typed()

if (readable_acceptor == nullptr)
{
LOG(ERROR) << "Failed to cast " << type_name() << " to " << "IReadableAcceptor<"
<< std::string(mrc::type_name<T>()) << ">" << "IReadableAcceptor<"
<< ::mrc::type_name(base.readable_acceptor_type().full_type()) << ">.";
LOG(ERROR) << "Failed to cast " << type_name() << " to "
<< "IReadableAcceptor<" << std::string(mrc::type_name<T>()) << ">"
<< "IReadableAcceptor<" << ::mrc::type_name(base.readable_acceptor_type().full_type()) << ">.";
throw exceptions::MrcRuntimeError("Failed to cast Sink to requested IReadableAcceptor<T>");
}

Expand All @@ -292,9 +292,9 @@ edge::IReadableProvider<T>& ObjectProperties::readable_provider_typed()

if (readable_provider == nullptr)
{
LOG(ERROR) << "Failed to cast " << type_name() << " to " << "IReadableProvider<"
<< std::string(mrc::type_name<T>()) << ">" << "IReadableProvider<"
<< ::mrc::type_name(base.readable_provider_type().full_type()) << ">.";
LOG(ERROR) << "Failed to cast " << type_name() << " to "
<< "IReadableProvider<" << std::string(mrc::type_name<T>()) << ">"
<< "IReadableProvider<" << ::mrc::type_name(base.readable_provider_type().full_type()) << ">.";
throw exceptions::MrcRuntimeError("Failed to cast Sink to requested IReadableProvider<T>");
}

Expand Down
5 changes: 5 additions & 0 deletions cpp/mrc/include/mrc/utils/tuple_utils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ void tuple_for_each(TupleT&& tuple, FuncT&& f)
std::make_index_sequence<std::tuple_size<std::decay_t<TupleT>>::value>());
}

// NOLINTBEGIN(readability-identifier-naming)
// Disable naming conventions for std library-like functions

/**
* @brief Creates a tuple of N elements of type T. For example, `repeat_tuple_type<int, 3>` would be `std::tuple<int,
* int, int>`
Expand Down Expand Up @@ -95,4 +98,6 @@ class repeat_tuple_type
template <typename T, size_t N>
using repeat_tuple_type_t = typename repeat_tuple_type<T, N>::type;

// NOLINTEND(readability-identifier-naming)

} // namespace mrc::utils
1 change: 0 additions & 1 deletion cpp/mrc/tests/node/operators/test_combine_latest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "../../test_mrc.hpp" // IWYU pragma: associated
#include "../test_nodes.hpp"
#include "gtest/gtest.h"

#include "mrc/node/operators/combine_latest.hpp"

Expand Down
57 changes: 36 additions & 21 deletions cpp/mrc/tests/node/operators/test_router.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "../../test_mrc.hpp" // IWYU pragma: associated
#include "../test_nodes.hpp"
#include "gtest/gtest.h"

#include "mrc/exceptions/runtime_error.hpp" // for MrcRuntimeError
#include "mrc/node/operators/router.hpp"
Expand Down Expand Up @@ -56,9 +55,9 @@ class DerivedRouterBase : public BaseT

void run()
{
constexpr bool has_do_run = requires(this_t& t) { t.do_run(); };
constexpr bool HasDoRun = requires(this_t& t) { t.do_run(); };

if constexpr (has_do_run)
if constexpr (HasDoRun)
{
this->do_run();
}
Expand All @@ -82,9 +81,9 @@ class DerivedLambdaRouter : public BaseT

void run()
{
constexpr bool has_do_run = requires(this_t& t) { t.do_run(); };
constexpr bool HasDoRun = requires(this_t& t) { t.do_run(); };

if constexpr (has_do_run)
if constexpr (HasDoRun)
{
this->do_run();
}
Expand Down Expand Up @@ -141,41 +140,57 @@ class TestRouterTypes : public testing::Test
}
};

using RouterTypes = ::testing::Types<node::DerivedRouterBase<int, node::StaticRouterComponentBase<int, int>>,
node::DerivedRouterBase<int, node::StaticRouterRunnableBase<int, int>>,
node::DerivedRouterBase<int, node::DynamicRouterComponentBase<int, int>>,
node::DerivedRouterBase<int, node::DynamicRouterRunnableBase<int, int>>,
node::DerivedLambdaRouter<node::LambdaStaticRouterComponent<int, int>>,
node::DerivedLambdaRouter<node::LambdaStaticRouterRunnable<int, int>>,
node::DerivedLambdaRouter<node::LambdaDynamicRouterComponent<int, int>>,
node::DerivedLambdaRouter<node::LambdaDynamicRouterRunnable<int, int>>>;
using router_types_t = ::testing::Types<node::DerivedRouterBase<int, node::StaticRouterComponentBase<int, int>>,
node::DerivedRouterBase<int, node::StaticRouterRunnableBase<int, int>>,
node::DerivedRouterBase<int, node::DynamicRouterComponentBase<int, int>>,
node::DerivedRouterBase<int, node::DynamicRouterRunnableBase<int, int>>,
node::DerivedLambdaRouter<node::LambdaStaticRouterComponent<int, int>>,
node::DerivedLambdaRouter<node::LambdaStaticRouterRunnable<int, int>>,
node::DerivedLambdaRouter<node::LambdaDynamicRouterComponent<int, int>>,
node::DerivedLambdaRouter<node::LambdaDynamicRouterRunnable<int, int>>>;

class RouterTypesNameGenerator
{
public:
template <typename T>
static std::string GetName(int)
static std::string GetName(int /*unused*/) // NOLINT(readability-identifier-naming)
{
if constexpr (std::is_same_v<T, node::DerivedRouterBase<int, node::StaticRouterComponentBase<int, int>>>)
{
return "StaticComponentBase";
}
if constexpr (std::is_same_v<T, node::DerivedRouterBase<int, node::StaticRouterRunnableBase<int, int>>>)
{
return "StaticRunnableBase";
}
if constexpr (std::is_same_v<T, node::DerivedRouterBase<int, node::DynamicRouterComponentBase<int, int>>>)
{
return "DynamicComponentBase";
}
if constexpr (std::is_same_v<T, node::DerivedRouterBase<int, node::DynamicRouterRunnableBase<int, int>>>)
{
return "DynamicRunnableBase";
}
if constexpr (std::is_same_v<T, node::DerivedLambdaRouter<node::LambdaStaticRouterComponent<int, int>>>)
{
return "LambdaStaticComponent";
}
if constexpr (std::is_same_v<T, node::DerivedLambdaRouter<node::LambdaStaticRouterRunnable<int, int>>>)
{
return "LambdaStaticRunnable";
}
if constexpr (std::is_same_v<T, node::DerivedLambdaRouter<node::LambdaDynamicRouterComponent<int, int>>>)
{
return "LambdaDynamicComponent";
}
if constexpr (std::is_same_v<T, node::DerivedLambdaRouter<node::LambdaDynamicRouterRunnable<int, int>>>)
{
return "LambdaDynamicRunnable";
}
}
};

TYPED_TEST_SUITE(TestRouterTypes, RouterTypes, RouterTypesNameGenerator);
TYPED_TEST_SUITE(TestRouterTypes, router_types_t, RouterTypesNameGenerator);

TYPED_TEST(TestRouterTypes, SourceToRouterToSinks)
{
Expand Down Expand Up @@ -243,9 +258,9 @@ TYPED_TEST(TestRouterTypes, AutomaticTypeConversion)
sink1->run();
sink2->run();

EXPECT_EQ((std::vector<float>{0.0f, 3.0f, 6.0f, 9.0f}), sink0->get_values());
EXPECT_EQ((std::vector<float>{1.0f, 4.0f, 7.0f}), sink1->get_values());
EXPECT_EQ((std::vector<float>{2.0f, 5.0f, 8.0f}), sink2->get_values());
EXPECT_EQ((std::vector<float>{0.0F, 3.0F, 6.0F, 9.0F}), sink0->get_values());
EXPECT_EQ((std::vector<float>{1.0F, 4.0F, 7.0F}), sink1->get_values());
EXPECT_EQ((std::vector<float>{2.0F, 5.0F, 8.0F}), sink2->get_values());
}

TYPED_TEST(TestRouterTypes, SourceComponentToRouterToDifferentSinks)
Expand All @@ -267,9 +282,9 @@ TYPED_TEST(TestRouterTypes, SourceComponentToRouterToDifferentSinks)
sink0->run();
sink2->run();

EXPECT_EQ((std::vector<float>{0.0f, 3.0f, 6.0f, 9.0f}), sink0->get_values());
EXPECT_EQ((std::vector<float>{1.0f, 4.0f, 7.0f}), sink1->get_values());
EXPECT_EQ((std::vector<float>{2.0f, 5.0f, 8.0f}), sink2->get_values());
EXPECT_EQ((std::vector<float>{0.0F, 3.0F, 6.0F, 9.0F}), sink0->get_values());
EXPECT_EQ((std::vector<float>{1.0F, 4.0F, 7.0F}), sink1->get_values());
EXPECT_EQ((std::vector<float>{2.0F, 5.0F, 8.0F}), sink2->get_values());
}
else
{
Expand Down
1 change: 0 additions & 1 deletion cpp/mrc/tests/node/operators/test_with_latest_from.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "../../test_mrc.hpp" // IWYU pragma: associated
#include "../test_nodes.hpp"
#include "gtest/gtest.h"

#include "mrc/node/operators/with_latest_from.hpp"
#include "mrc/node/operators/zip.hpp"
Expand Down
1 change: 0 additions & 1 deletion cpp/mrc/tests/node/operators/test_zip.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "../../test_mrc.hpp" // IWYU pragma: associated
#include "../test_nodes.hpp"
#include "gtest/gtest.h"

#include "mrc/exceptions/runtime_error.hpp" // for MrcRuntimeError
#include "mrc/node/operators/zip.hpp"
Expand Down
50 changes: 27 additions & 23 deletions cpp/mrc/tests/node/test_nodes.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

#pragma once

#include "../test_mrc.hpp" // IWYU pragma: associated

#include "mrc/channel/buffered_channel.hpp" // IWYU pragma: keep
Expand Down Expand Up @@ -65,6 +67,31 @@ using namespace std::chrono_literals;

namespace mrc::node {

template <size_t N, typename... T>
typename std::enable_if<(N >= sizeof...(T))>::type print_tuple(std::ostream& /*unused*/,
const std::tuple<T...>& /*unused*/)
{}

template <size_t N, typename... T>
typename std::enable_if<(N < sizeof...(T))>::type print_tuple(std::ostream& os, const std::tuple<T...>& tup)
{
if (N != 0)
{
os << ", ";
}
os << std::get<N>(tup);
print_tuple<N + 1>(os, tup);
}

// Utility function to print tuples
template <typename... T>
std::ostream& operator<<(std::ostream& os, const std::tuple<T...>& tup)
{
os << "[";
print_tuple<0>(os, tup);
return os << "]";
}

template <typename T>
class EdgeReadableLambda : public edge::IEdgeReadable<T>
{
Expand Down Expand Up @@ -457,27 +484,4 @@ class TestDynamicRouter : public DynamicRouterComponentBase<std::string, T>
return keys[t % keys.size()];
}
};

template <size_t n, typename... T>
typename std::enable_if<(n >= sizeof...(T))>::type print_tuple(std::ostream&, const std::tuple<T...>&)
{}

template <size_t n, typename... T>
typename std::enable_if<(n < sizeof...(T))>::type print_tuple(std::ostream& os, const std::tuple<T...>& tup)
{
if (n != 0)
os << ", ";
os << std::get<n>(tup);
print_tuple<n + 1>(os, tup);
}

// Utility function to print tuples
template <typename... T>
std::ostream& operator<<(std::ostream& os, const std::tuple<T...>& tup)
{
os << "[";
print_tuple<0>(os, tup);
return os << "]";
}

} // namespace mrc::node
18 changes: 9 additions & 9 deletions python/mrc/core/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ PYBIND11_MODULE(node, py_mod)
{
return make_node.template operator()<1>(name);
}
else if (count == 2)
if (count == 2)
{
return make_node.template operator()<2>(name);
}
else if (count == 3)
if (count == 3)
{
return make_node.template operator()<3>(name);
}
else if (count == 4)
if (count == 4)

Check warning on line 125 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L125 

Added line #L125 was not covered by tests
{
return make_node.template operator()<4>(name);

Check warning on line 127 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L127 

Added line #L127 was not covered by tests
}
Expand Down Expand Up @@ -176,15 +176,15 @@ PYBIND11_MODULE(node, py_mod)
{
return make_node.template operator()<1>(name);
}
else if (count == 2)
if (count == 2)
{
return make_node.template operator()<2>(name);
}
else if (count == 3)
if (count == 3)
{
return make_node.template operator()<3>(name);
}
else if (count == 4)
if (count == 4)

Check warning on line 187 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L187 

Added line #L187 was not covered by tests
{
return make_node.template operator()<4>(name);

Check warning on line 189 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L189 

Added line #L189 was not covered by tests
}
Expand Down Expand Up @@ -237,15 +237,15 @@ PYBIND11_MODULE(node, py_mod)
{
return make_node.template operator()<1>(name);
}
else if (count == 2)
if (count == 2)
{
return make_node.template operator()<2>(name);
}
else if (count == 3)
if (count == 3)
{
return make_node.template operator()<3>(name);
}
else if (count == 4)
if (count == 4)

Check warning on line 248 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L248 

Added line #L248 was not covered by tests
{
return make_node.template operator()<4>(name);

Check warning on line 250 in python/mrc/core/node.cpp

View check run for this annotation

Codecov / codecov/patch

python/mrc/core/node.cpp#L250 

Added line #L250 was not covered by tests
}
Expand Down
Loading

0 comments on commit 8554c66

Please sign in to comment.