Skip to content

Commit 53dc1d0

Browse files
MorganCaronMorganCaron
committed
Container/NetworkPtr: Fix detach
1 parent c968f12 commit 53dc1d0

2 files changed

Lines changed: 148 additions & 45 deletions

File tree

‎modules/Container/NetworkPtr.mpp‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export namespace CppUtils::Container
7171

7272
inline auto setWeakThis(WeakPtr ptr) noexcept -> void
7373
{
74-
m_weak_this = std::move(ptr);
74+
m_weakThis = std::move(ptr);
7575
}
7676

7777
[[nodiscard]] inline auto getDistanceFromRoot() const noexcept -> std::size_t
@@ -91,21 +91,24 @@ export namespace CppUtils::Container
9191
m_children.push_back(childPtr);
9292
else
9393
m_children.push_back(WeakPtr{childPtr});
94-
childValue.m_parents.push_back(m_weak_this);
94+
childValue.m_parents.push_back(m_weakThis);
9595
}
9696

9797
inline auto detachChild(SharedPtr& childPtr, NetworkPtr& childValue) -> void
9898
{
99-
if (auto thisPtr = m_weak_this.lock(); thisPtr)
100-
if (auto childIt = std::find_if(std::cbegin(m_children), std::cend(m_children), [&childPtr](const auto& child) -> bool {
99+
if (auto thisPtr = m_weakThis.lock(); thisPtr)
100+
{
101+
const auto children = m_children | std::views::reverse;
102+
if (auto childIt = std::ranges::find_if(children, [&childPtr](const auto& child) -> bool {
101103
return std::visit([&childPtr](const auto& child) -> bool {
102104
return Thread::SharedPtr::ownerEqual(childPtr, child);
103105
}, child);
104106
});
105-
childIt != std::cend(m_children))
107+
childIt != std::ranges::end(children))
106108
{
107-
auto mustUpdateDistance = std::holds_alternative<SharedPtr>(*childIt);
108-
m_children.erase(childIt);
109+
auto baseIt = std::prev(childIt.base());
110+
auto needsDistanceUpdate = std::holds_alternative<SharedPtr>(*baseIt);
111+
m_children.erase(baseIt);
109112
auto& parents = childValue.m_parents;
110113
if (auto parentIt = std::find_if(
111114
std::cbegin(parents),
@@ -116,10 +119,11 @@ export namespace CppUtils::Container
116119
parentIt != std::cend(parents))
117120
{
118121
parents.erase(parentIt);
119-
if (mustUpdateDistance)
120-
childValue.updateDistance(m_distanceFromRoot);
122+
if (needsDistanceUpdate)
123+
childValue.updateDistance(m_weakThis, m_distanceFromRoot);
121124
}
122125
}
126+
}
123127
}
124128

125129
inline auto detachChildren() -> void
@@ -141,7 +145,7 @@ export namespace CppUtils::Container
141145

142146
inline auto detachParent(SharedPtr& parent) -> void
143147
{
144-
if (auto thisPtr = m_weak_this.lock(); thisPtr)
148+
if (auto thisPtr = m_weakThis.lock(); thisPtr)
145149
{
146150
auto accessor = Thread::MultipleAccessor{*parent, *thisPtr};
147151
auto& parentRef = std::get<0>(accessor.values);
@@ -152,7 +156,7 @@ export namespace CppUtils::Container
152156

153157
inline auto detachParents() -> void
154158
{
155-
if (auto thisPtr = m_weak_this.lock(); thisPtr)
159+
if (auto thisPtr = m_weakThis.lock(); thisPtr)
156160
for (const auto& parentWeak : m_parents)
157161
if (auto parent = parentWeak.lock())
158162
{
@@ -164,57 +168,54 @@ export namespace CppUtils::Container
164168
}
165169

166170
private:
167-
static inline constexpr auto getChildAccessor(auto& child) -> decltype(auto)
171+
static inline constexpr auto getChildAccessor(auto& child) -> std::optional<Accessor>
168172
{
169173
auto sharedPtr = SharedPtr{};
170174
if (std::holds_alternative<SharedPtr>(child))
171175
sharedPtr = std::get<SharedPtr>(child);
172176
else
173177
sharedPtr = SharedPtr{std::get<WeakPtr>(child)};
178+
if (not sharedPtr)
179+
return std::nullopt;
174180
return sharedPtr->uniqueAccess();
175181
}
176182

177-
inline auto updateDistance() -> void
183+
inline auto updateDistance(const WeakPtr& lockedParentWeak = {}, std::size_t lockedParentDistance = std::numeric_limits<std::size_t>::max()) -> void
178184
{
179-
if (m_isRoot)
180-
{
181-
m_distanceFromRoot = 0;
182-
return;
183-
}
184-
185185
auto oldDistanceFromRoot = m_distanceFromRoot;
186186

187187
m_distanceFromRoot = std::numeric_limits<std::size_t>::max();
188188
for (const auto& parentWeak : m_parents)
189-
if (auto parent = parentWeak.lock(); parent)
190-
if (auto accessor = parent->sharedAccess();
191-
accessor->getDistanceFromRoot() + 1 < m_distanceFromRoot)
192-
m_distanceFromRoot = accessor->getDistanceFromRoot() + 1;
189+
{
190+
auto parentDistance = std::numeric_limits<std::size_t>::max();
191+
if (Thread::SharedPtr::ownerEqual(parentWeak, lockedParentWeak))
192+
parentDistance = lockedParentDistance;
193+
else if (auto parent = parentWeak.lock())
194+
{
195+
auto accessor = parent->sharedAccess();
196+
parentDistance = accessor->getDistanceFromRoot();
197+
}
198+
199+
if (parentDistance != std::numeric_limits<std::size_t>::max() && parentDistance + 1 < m_distanceFromRoot)
200+
m_distanceFromRoot = parentDistance + 1;
201+
}
202+
193203
if (m_distanceFromRoot == oldDistanceFromRoot)
194204
return;
195205

196206
for (auto& child : m_children)
197207
{
198-
auto accessor = getChildAccessor(child);
199-
if (m_distanceFromRoot < oldDistanceFromRoot)
200-
accessor->updateDistance(m_distanceFromRoot);
201-
else
202-
accessor->updateDistance();
203-
}
204-
}
208+
auto childWeak = WeakPtr{};
209+
if (const auto* shared = std::get_if<SharedPtr>(&child))
210+
childWeak = *shared;
211+
else if (const auto* weak = std::get_if<WeakPtr>(&child))
212+
childWeak = *weak;
205213

206-
inline auto updateDistance(std::size_t parentDistance) -> void
207-
{
208-
if (m_isRoot)
209-
return;
210-
if (m_distanceFromRoot < parentDistance + 1)
211-
{
212-
m_distanceFromRoot = parentDistance + 1;
213-
for (auto& child : m_children)
214-
{
215-
auto accessor = getChildAccessor(child);
216-
accessor->updateDistance(m_distanceFromRoot);
217-
}
214+
if (Thread::SharedPtr::ownerEqual(childWeak, lockedParentWeak))
215+
continue;
216+
217+
if (auto accessor = getChildAccessor(child))
218+
(*accessor)->updateDistance(m_weakThis, m_distanceFromRoot);
218219
}
219220
}
220221

@@ -224,7 +225,7 @@ export namespace CppUtils::Container
224225
private:
225226
bool m_isRoot = false;
226227
std::size_t m_distanceFromRoot = std::numeric_limits<std::size_t>::max();
227-
WeakPtr m_weak_this;
228+
WeakPtr m_weakThis;
228229
std::vector<std::variant<SharedPtr, WeakPtr>> m_children;
229230
std::vector<WeakPtr> m_parents;
230231
};

‎tests/Container/NetworkPtr.mpp‎

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,22 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
1111
using StringNetworkPtr = CppUtils::Container::NetworkPtr<std::string>;
1212

1313
suite.addTest("Create root", [&] {
14-
auto _ = NetworkPtr::makeRoot("Root");
14+
auto root = NetworkPtr::makeRoot("Root");
15+
auto accessor = root->sharedAccess();
16+
suite.expectEqual(accessor->getDistanceFromRoot(), 0uz);
1517
});
1618

1719
suite.addTest("Create node", [&] {
18-
auto _ = NetworkPtr::make("Node");
20+
auto node = NetworkPtr::make("Node");
21+
auto accessor = node->sharedAccess();
22+
suite.expectEqual(accessor->getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
1923
});
2024

2125
suite.addTest("Read value", [&] {
2226
auto node = StringNetworkPtr::make("Node");
2327
{
2428
auto accessor = node->sharedAccess();
29+
suite.expectEqual(accessor->getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
2530
suite.expectEqual(accessor->value, "Node");
2631
}
2732
});
@@ -45,7 +50,9 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
4550
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *branch};
4651
auto& rootRef = std::get<0>(accessor.values);
4752
auto& branchRef = std::get<1>(accessor.values);
53+
suite.expectEqual(branchRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
4854
rootRef.attachChild(branch, branchRef);
55+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
4956
}
5057
Logger::print("Persistance de Branch\n");
5158
});
@@ -59,19 +66,46 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
5966
auto& rootRef = std::get<0>(accessor.values);
6067
auto& branchRef = std::get<1>(accessor.values);
6168
rootRef.attachChild(branch, branchRef);
69+
suite.expectEqual(rootRef.getDistanceFromRoot(), 0uz);
70+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
6271
}
6372
{
6473
auto leaf = NetworkPtr::make("Leaf", 2uz);
6574
auto accessor = CppUtils::Thread::MultipleAccessor{*branch, *leaf};
6675
auto& branchRef = std::get<0>(accessor.values);
6776
auto& leafRef = std::get<1>(accessor.values);
6877
branchRef.attachChild(leaf, leafRef);
78+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
79+
suite.expectEqual(leafRef.getDistanceFromRoot(), 2uz);
6980
}
7081
Logger::print("Persistance de Leaf\n");
7182
}
7283
Logger::print("Persistance de Branch\n");
7384
});
7485

86+
suite.addTest("Cut leaf", [&] {
87+
auto root = NetworkPtr::makeRoot("Root");
88+
{
89+
auto leaf = NetworkPtr::make("Leaf", 1uz);
90+
{
91+
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *leaf};
92+
auto& rootRef = std::get<0>(accessor.values);
93+
auto& leafRef = std::get<1>(accessor.values);
94+
suite.expectEqual(leafRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
95+
rootRef.attachChild(leaf, leafRef);
96+
suite.expectEqual(leafRef.getDistanceFromRoot(), 1uz);
97+
}
98+
Logger::print("Persistance de Leaf\n");
99+
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *leaf};
100+
auto& rootRef = std::get<0>(accessor.values);
101+
auto& leafRef = std::get<1>(accessor.values);
102+
suite.expectEqual(leafRef.getDistanceFromRoot(), 1uz);
103+
rootRef.detachChild(leaf, leafRef);
104+
suite.expectEqual(leafRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
105+
}
106+
Logger::print("Non-persistance de Leaf\n");
107+
});
108+
75109
suite.addTest("Cut branch", [&] {
76110
auto root = NetworkPtr::makeRoot("Root");
77111
{
@@ -93,7 +127,9 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
93127
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *branch};
94128
auto& rootRef = std::get<0>(accessor.values);
95129
auto& branchRef = std::get<1>(accessor.values);
130+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
96131
rootRef.detachChild(branch, branchRef);
132+
suite.expectEqual(branchRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
97133
}
98134
Logger::print("Non-persistance de Branch\n");
99135
});
@@ -107,6 +143,8 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
107143
auto& rootRef = std::get<0>(accessor.values);
108144
auto& branchRef = std::get<1>(accessor.values);
109145
rootRef.attachChild(branch, branchRef);
146+
suite.expectEqual(rootRef.getDistanceFromRoot(), 0uz);
147+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
110148
}
111149
Logger::print("Persistance de Branch\n");
112150
{
@@ -115,8 +153,72 @@ export namespace CppUtils::UnitTest::Container::NetworkPtr
115153
auto& branchRef = std::get<1>(accessor.values);
116154
Logger::print("Création d'une boucle\n");
117155
branchRef.attachChild(root, rootRef);
156+
suite.expectEqual(rootRef.getDistanceFromRoot(), 0uz);
157+
suite.expectEqual(branchRef.getDistanceFromRoot(), 1uz);
118158
}
119159
Logger::print("Libération de la boucle\n");
120160
});
161+
162+
suite.addTest("Detach child with multiple links", [&] {
163+
auto root = NetworkPtr::makeRoot("Root");
164+
{
165+
auto leaf = NetworkPtr::make("Leaf", 1uz);
166+
{
167+
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *leaf};
168+
auto& rootRef = std::get<0>(accessor.values);
169+
auto& leafRef = std::get<1>(accessor.values);
170+
suite.expectEqual(leafRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
171+
172+
rootRef.attachChild(leaf, leafRef);
173+
suite.expectEqual(leafRef.getDistanceFromRoot(), 1uz);
174+
175+
rootRef.attachChild(leaf, leafRef);
176+
suite.expectEqual(leafRef.getDistanceFromRoot(), 1uz);
177+
178+
rootRef.detachChild(leaf, leafRef);
179+
suite.expectEqual(leafRef.getDistanceFromRoot(), 1uz);
180+
181+
rootRef.detachChild(leaf, leafRef);
182+
suite.expectEqual(leafRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
183+
}
184+
Logger::print("Non-persistance de Leaf\n");
185+
}
186+
Logger::print("Persistance de Branch\n");
187+
});
188+
189+
suite.addTest("Attach and detach root on itself", [&] {
190+
auto root = NetworkPtr::makeRoot("Root");
191+
auto accessor = root->uniqueAccess();
192+
accessor->attachChild(root, accessor.value());
193+
suite.expectEqual(accessor->getDistanceFromRoot(), 0uz);
194+
accessor->detachChild(root, accessor.value());
195+
suite.expectEqual(accessor->getDistanceFromRoot(), 0uz);
196+
});
197+
198+
suite.addTest("Attach and detach node on itself", [&] {
199+
auto root = NetworkPtr::makeRoot("Root");
200+
auto node = NetworkPtr::make("Node", 1uz);
201+
{
202+
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *node};
203+
auto& rootRef = std::get<0>(accessor.values);
204+
auto& nodeRef = std::get<1>(accessor.values);
205+
rootRef.attachChild(node, nodeRef);
206+
suite.expectEqual(nodeRef.getDistanceFromRoot(), 1uz);
207+
}
208+
{
209+
auto accessor = node->uniqueAccess();
210+
accessor->attachChild(node, accessor.value());
211+
suite.expectEqual(accessor->getDistanceFromRoot(), 1uz);
212+
accessor->detachChild(node, accessor.value());
213+
suite.expectEqual(accessor->getDistanceFromRoot(), 1uz);
214+
}
215+
{
216+
auto accessor = CppUtils::Thread::MultipleAccessor{*root, *node};
217+
auto& rootRef = std::get<0>(accessor.values);
218+
auto& nodeRef = std::get<1>(accessor.values);
219+
rootRef.detachChild(node, nodeRef);
220+
suite.expectEqual(nodeRef.getDistanceFromRoot(), std::numeric_limits<std::size_t>::max());
221+
}
222+
});
121223
}};
122224
}

0 commit comments

Comments
 (0)