Skip to content

Commit

Permalink
Chore: improve concept definitions (#521)
Browse files Browse the repository at this point in the history
* Clean up concept definitions
* Fix ParallelTests/NodeMultiThread bug

Authors:
  - Will Killian (https://github.com/willkill07)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #521
  • Loading branch information
willkill07 authored Dec 3, 2024
1 parent 10439b3 commit 7961e5a
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 12 deletions.
4 changes: 2 additions & 2 deletions cpp/mrc/include/mrc/core/concepts/not_void.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-FileCopyrightText: Copyright (c) 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -22,6 +22,6 @@
namespace mrc::core::concepts {

template <typename T>
concept not_void = requires { requires not std::same_as<T, void>; };
concept not_void = (not std::same_as<T, void>);

} // namespace mrc::core::concepts
21 changes: 12 additions & 9 deletions cpp/mrc/include/mrc/coroutines/concepts/awaitable.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-FileCopyrightText: Copyright (c) 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -44,6 +44,14 @@
#include <utility>

namespace mrc::coroutines::concepts {

// clang-format off
/**
* The concept declares a type that is required to be one of the passed types
*/
template<typename T, typename ... CandidatesT>
concept one_of = (... or std::same_as<T, CandidatesT>);

/**
* This concept declares a type that is required to meet the c++20 coroutine operator co_await()
* retun type. It requires the following three member functions:
Expand All @@ -52,14 +60,11 @@ namespace mrc::coroutines::concepts {
* await_resume() -> decltype(auto)
* Where the return type on await_resume is the requested return of the awaitable.
*/
// clang-format off
template<typename T>
concept awaiter = requires(T t, std::coroutine_handle<> c)
{
{ t.await_ready() } -> std::same_as<bool>;
requires std::same_as<decltype(t.await_suspend(c)), void> ||
std::same_as<decltype(t.await_suspend(c)), bool> ||
std::same_as<decltype(t.await_suspend(c)), std::coroutine_handle<>>;
{ t.await_suspend(c) } -> one_of<void, bool, std::coroutine_handle<>>;
{ t.await_resume() };
};

Expand All @@ -77,10 +82,8 @@ template<typename T>
concept awaiter_void = requires(T t, std::coroutine_handle<> c)
{
{ t.await_ready() } -> std::same_as<bool>;
requires std::same_as<decltype(t.await_suspend(c)), void> ||
std::same_as<decltype(t.await_suspend(c)), bool> ||
std::same_as<decltype(t.await_suspend(c)), std::coroutine_handle<>>;
{t.await_resume()} -> std::same_as<void>;
{ t.await_suspend(c) } -> one_of<void, bool, std::coroutine_handle<>>;
{ t.await_resume() } -> std::same_as<void>;
};

template<typename T>
Expand Down
3 changes: 2 additions & 1 deletion cpp/mrc/tests/test_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "mrc/segment/object.hpp"
#include "mrc/utils/string_utils.hpp"

#include <boost/fiber/operations.hpp>
#include <glog/logging.h>
#include <gtest/gtest.h>
#include <rxcpp/rx.hpp>
Expand Down Expand Up @@ -743,7 +744,7 @@ TEST_P(ParallelTests, NodeMultiThread)
}

DVLOG(1) << context.info() << " Node got value: '" << x << "'" << std::endl;

boost::this_fiber::yield();
EXPECT_TRUE(parallel_test.wait_for(250ms));
// Double the value
return x * 2;
Expand Down

0 comments on commit 7961e5a

Please sign in to comment.