Skip to content

Commit e4324a1

Browse files
author
Ivan Le Lann
committed
more test OK, zero copy, no default constructible
1 parent 9e6cc41 commit e4324a1

7 files changed

Lines changed: 57 additions & 67 deletions

File tree

‎include/react/Signal.h‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,22 @@ template <typename... TSignals> struct DomainTrait<SignalPack<TSignals...>> {
7070
template <typename S, typename F, typename... Args>
7171
using functor_t = FunctionOp<S, F, Args...>;
7272

73-
template <typename D, typename S, typename F, typename... Args>
74-
using signal_node_t = SignalOpNode<D, S, functor_t<S, F, std::decay_t<Args>...>>;
73+
template <typename D, typename S, typename TOp>
74+
using signal_op_node_t = SignalOpNode<D, S, TOp>;
7575

7676
template <typename D, typename S, typename F, typename... Args>
77-
auto make_node(F &&f, Args &&... args) {
78-
return std::make_shared<signal_node_t<D, S, F, Args...>>(
79-
std::forward<F>(f), std::forward<Args>(args)...);
77+
auto make_op_node(F &&f, Args &&... args) {
78+
using TOp = functor_t<S, F, std::decay_t<Args>...>;
79+
return std::make_shared<signal_op_node_t<D, S, TOp>>(
80+
TOp{std::forward<F>(f), std::forward<Args>(args)...});
8081
}
8182

8283
template <typename F, typename... TSignals>
8384
auto make_op_signal(F &&f, TSignals &&... args) {
8485
using D = domain_t<TSignals...>;
8586
using S = std::result_of_t<F(value_t<TSignals>...)>;
8687

87-
return Signal<D, S>{make_node<D, S>(
88+
return Signal<D, S>{make_op_node<D, S>(
8889
std::forward<F>(f), GetNodePtr(std::forward<TSignals>(args))...)};
8990
}
9091

@@ -155,14 +156,14 @@ template <typename D, typename V, typename FIn, typename F = std::decay_t<FIn>,
155156
typename S = std::result_of_t<F(V)>>
156157
auto MakeSignal(const Signal<D, V> &arg, FIn &&func) {
157158
return Signal<D, S>(
158-
impl::make_node<D, S>(std::forward<FIn>(func), GetNodePtr(arg)));
159+
impl::make_op_node<D, S>(std::forward<FIn>(func), GetNodePtr(arg)));
159160
}
160161

161162
// Multiple args
162163
template <typename D, typename S, typename F, typename Tuple, std::size_t... I>
163164
auto MakeNode(F &&f, const Tuple &tuple, std::index_sequence<I...>) {
164-
return impl::make_node<D, S>(std::forward<F>(f),
165-
GetNodePtr(std::get<I>(tuple))...);
165+
return impl::make_op_node<D, S>(std::forward<F>(f),
166+
GetNodePtr(std::get<I>(tuple))...);
166167
}
167168

168169
// Multiple args
@@ -177,7 +178,7 @@ auto MakeSignal(const SignalPack<TSignals...> &argPack, FIn &&func) {
177178
}
178179

179180
///////////////////////////////////////////////////////////////////////////////////////////////////
180-
/// Utils
181+
/// ValueTypeTrait
181182
///////////////////////////////////////////////////////////////////////////////////////////////////
182183

183184
template <typename S> struct ValueTypeTrait { using type = S; };
@@ -198,7 +199,7 @@ template <typename D, typename S> struct Signal {
198199
using DomainT = D;
199200

200201
Signal() : ptr_{} {}
201-
explicit Signal(NodePtrT p) : ptr_{p} {}
202+
explicit Signal(NodePtrT p) : ptr_{std::move(p)} {}
202203

203204
// Signal(const Signal &) = delete;
204205
// Signal& operator =(const Signal &) = delete;
@@ -251,7 +252,7 @@ auto operator+(const Signal<D, LV> &l, RV &&r) {
251252

252253
template <typename D, typename LV, typename RV,
253254
class = impl::if_not_signal_t<LV>>
254-
auto operator+(LV &&l, const Signal<D, RV> &&r) {
255+
auto operator+(LV &&l, const Signal<D, RV> &r) {
255256
return impl::make_op_signal(
256257
std::bind(std::plus<>(), std::forward<LV>(l), impl::_1), r);
257258
}
@@ -270,7 +271,7 @@ auto operator-(const Signal<D, LV> &l, RV &&r) {
270271

271272
template <typename D, typename LV, typename RV,
272273
class = impl::if_not_signal_t<LV>>
273-
auto operator-(LV &&l, const Signal<D, RV> &&r) {
274+
auto operator-(LV &&l, const Signal<D, RV> &r) {
274275
return impl::make_op_signal(
275276
std::bind(std::minus<>(), std::forward<LV>(l), impl::_1), r);
276277
}
@@ -289,7 +290,7 @@ auto operator*(const Signal<D, LV> &l, RV &&r) {
289290

290291
template <typename D, typename LV, typename RV,
291292
class = impl::if_not_signal_t<LV>>
292-
auto operator*(LV &&l, const Signal<D, RV> &&r) {
293+
auto operator*(LV &&l, const Signal<D, RV> &r) {
293294
return impl::make_op_signal(
294295
std::bind(std::multiplies<>(), std::forward<LV>(l), impl::_1), r);
295296
}
@@ -308,14 +309,14 @@ auto operator/(const Signal<D, LV> &l, RV &&r) {
308309

309310
template <typename D, typename LV, typename RV,
310311
class = impl::if_not_signal_t<LV>>
311-
auto operator/(LV &&l, const Signal<D, RV> &&r) {
312+
auto operator/(LV &&l, const Signal<D, RV> &r) {
312313
return impl::make_op_signal(
313314
std::bind(std::divides<>(), std::forward<LV>(l), impl::_1), r);
314315
}
315316

316317
template <typename D, typename S>
317318
Signal<D, S> operator-(const Signal<D, S> &in) {
318-
return in * S{-1};
319+
return S{-1} * in;
319320
}
320321

321322
template <typename D, typename S>

‎include/react/detail/graph/AlgorithmNodes.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,4 +657,4 @@ class PulseNode : public EventStreamNode<D,S>
657657

658658
/****************************************/ REACT_IMPL_END /***************************************/
659659

660-
#endif // REACT_DETAIL_GRAPH_ALGORITHMNODES_H_INCLUDED
660+
#endif // REACT_DETAIL_GRAPH_ALGORITHMNODES_H_INCLUDED

‎include/react/detail/graph/ContinuationNodes.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,4 +348,4 @@ class SyncedContinuationNode : public ContinuationNode<D>
348348

349349
/****************************************/ REACT_IMPL_END /***************************************/
350350

351-
#endif // REACT_DETAIL_GRAPH_CONTINUATIONNODES_H_INCLUDED
351+
#endif // REACT_DETAIL_GRAPH_CONTINUATIONNODES_H_INCLUDED

‎include/react/detail/graph/GraphBase.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ template <typename ... TDeps>
199199
class ReactiveOpBase
200200
{
201201
public:
202-
using DepHolderT = std::tuple<TDeps...>;
202+
using DepHolderT = std::tuple<std::decay_t<TDeps>...>;
203203

204204
template <typename ... TDepsIn>
205205
ReactiveOpBase(DontMove, TDepsIn&& ... deps) :

‎include/react/detail/graph/SignalNodes.h‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include <memory>
1515
#include <utility>
16+
#include <type_traits>
1617

1718
#include "GraphBase.h"
1819

@@ -35,8 +36,6 @@ template
3536
class SignalNode : public ObservableNode<D>
3637
{
3738
public:
38-
// TODO why ?
39-
SignalNode() = default;
4039

4140
template <typename T>
4241
explicit SignalNode(T&& value) :
@@ -72,9 +71,7 @@ class VarNode :
7271
public:
7372
template <typename T>
7473
VarNode(T&& value) :
75-
VarNode::SignalNode( std::forward<T>(value) ),
76-
// TODO assign newValue_ with value ? value is moved-from just above
77-
newValue_( value )
74+
VarNode::SignalNode( std::forward<T>(value) )
7875
{
7976
Engine::OnNodeCreate(*this);
8077
}
@@ -96,7 +93,7 @@ class VarNode :
9693
template <typename V>
9794
void AddInput(V&& newValue)
9895
{
99-
newValue_ = std::forward<V>(newValue);
96+
pushNewValue(std::forward<V>(newValue));
10097

10198
isInputAdded_ = true;
10299

@@ -121,7 +118,7 @@ class VarNode :
121118
// in ApplyInput
122119
else
123120
{
124-
func(newValue_);
121+
func(getNewValue());
125122
}
126123
}
127124

@@ -134,9 +131,9 @@ class VarNode :
134131
{
135132
isInputAdded_ = false;
136133

137-
if (! Equals(this->value_, newValue_))
134+
if (! Equals(this->value_, getNewValue()))
138135
{
139-
this->value_ = std::move(newValue_);
136+
popNewValue();
140137
Engine::OnInputChange(*this, turn);
141138
return true;
142139
}
@@ -160,7 +157,26 @@ class VarNode :
160157
}
161158

162159
private:
163-
S newValue_;
160+
161+
S & getNewValue()
162+
{
163+
return *reinterpret_cast<S*>(newValueData);
164+
}
165+
166+
template <typename V>
167+
void pushNewValue(V && v)
168+
{
169+
new (newValueData) S{std::forward<V>(v)};
170+
}
171+
172+
void popNewValue()
173+
{
174+
this->value_ = std::move(*reinterpret_cast<S*>(newValueData));
175+
}
176+
177+
// so that we don't require S to be default constructible
178+
std::aligned_storage_t<sizeof(S), alignof(S)> newValueData[1];
179+
164180
bool isInputAdded_ = false;
165181
bool isInputModified_ = false;
166182
};
@@ -183,11 +199,6 @@ class FunctionOp : public ReactiveOpBase<TDeps...>
183199
func_( std::forward<FIn>(func) )
184200
{}
185201

186-
FunctionOp(FunctionOp&& other) :
187-
FunctionOp::ReactiveOpBase( std::move(other) ),
188-
func_( std::move(other.func_) )
189-
{}
190-
191202
S Evaluate()
192203
{
193204
return apply(EvalFunctor( func_ ), this->deps_);
@@ -239,13 +250,10 @@ class SignalOpNode :
239250
using Engine = typename SignalOpNode::Engine;
240251

241252
public:
242-
template <typename ... TArgs>
243-
SignalOpNode(TArgs&& ... args) :
244-
SignalOpNode::SignalNode( ),
245-
op_( std::forward<TArgs>(args) ... )
253+
explicit SignalOpNode(TOp op) :
254+
SignalOpNode::SignalNode{op.Evaluate()},
255+
op_{std::move(op)}
246256
{
247-
this->value_ = op_.Evaluate();
248-
249257
Engine::OnNodeCreate(*this);
250258
op_.template Attach<D>(*this);
251259
}

‎tests/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ endfunction(cppreact_test)
1919
#cppreact_test(NAME EventStreamTest SOURCES src/EventStreamTest.cpp)
2020
#cppreact_test(NAME EventStreamTestQ SOURCES src/EventStreamTestQ.cpp)
2121
cppreact_test(NAME MoveTest SOURCES src/MoveTest.cpp)
22-
#cppreact_test(NAME ObserverTest SOURCES src/ObserverTest.cpp)
22+
cppreact_test(NAME ObserverTest SOURCES src/ObserverTest.cpp)
2323
#cppreact_test(NAME ObserverTestQ SOURCES src/ObserverTestQ.cpp)
2424
#cppreact_test(NAME OperationsTest SOURCES src/OperationsTest.cpp)
2525
#cppreact_test(NAME OperationsTestQ SOURCES src/OperationsTestQ.cpp)

‎tests/src/MoveTest.h‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class MoveTest : public testing::Test
3030

3131
struct Stats
3232
{
33-
int copyCount = 0;
3433
int moveCount = 0;
3534
};
3635

@@ -39,19 +38,13 @@ class MoveTest : public testing::Test
3938
int v = 0;
4039
Stats* stats = nullptr;
4140

42-
CopyCounter() = default;
43-
4441
CopyCounter(int x, Stats* s) :
4542
v( x ),
4643
stats( s )
4744
{}
4845

49-
CopyCounter(const CopyCounter& other) :
50-
v( other.v ),
51-
stats( other.stats )
52-
{
53-
stats->copyCount++;
54-
}
46+
CopyCounter(const CopyCounter& other) = delete;
47+
CopyCounter& operator=(const CopyCounter& other) = delete;
5548

5649
CopyCounter(CopyCounter&& other) :
5750
v( other.v ),
@@ -60,14 +53,6 @@ class MoveTest : public testing::Test
6053
stats->moveCount++;
6154
}
6255

63-
CopyCounter& operator=(const CopyCounter& other)
64-
{
65-
v = other.v;
66-
stats = other.stats;
67-
stats->copyCount++;
68-
return *this;
69-
}
70-
7156
CopyCounter& operator=(CopyCounter&& other)
7257
{
7358
v = other.v;
@@ -107,21 +92,17 @@ TYPED_TEST_P(MoveTest, Copy1)
10792
auto d = MakeVar<D>(CopyCounter{1000,&stats1});
10893

10994
// 4x move to value_
110-
// 4x copy to newValue_ (can't be unitialized for references)
111-
ASSERT_EQ(stats1.copyCount, 4);
112-
ASSERT_EQ(stats1.moveCount, 4);
95+
ASSERT_EQ(4, stats1.moveCount);
11396

11497
auto x = a + b + c + d;
11598

116-
ASSERT_EQ(stats1.copyCount, 4);
117-
ASSERT_EQ(stats1.moveCount, 7);
118-
ASSERT_EQ(x().v, 1111);
99+
ASSERT_EQ(7, stats1.moveCount);
100+
ASSERT_EQ(1111, x().v);
119101

120102
a <<= CopyCounter{2,&stats1};
121103

122-
ASSERT_EQ(stats1.copyCount, 4);
123-
ASSERT_EQ(stats1.moveCount, 10);
124-
ASSERT_EQ(x().v, 1112);
104+
ASSERT_EQ(10, stats1.moveCount);
105+
ASSERT_EQ(1112, x().v);
125106
}
126107

127108
///////////////////////////////////////////////////////////////////////////////////////////////////

0 commit comments

Comments
 (0)