From fad97b35430fcfb1ef963c0107c7b0de66a8b5e2 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Mon, 28 Jun 2021 23:52:47 +0000 Subject: [PATCH] crimson: introduce assert_moveable(). In C++ `std::moving` a `const`-qualified value yields a constant r-value reference (`const T&&`) which won't be matched with a callable taking non-constant r-value reference (like move constructors) but can play with one taking a constant l-value reference (like copy constructors do). This behaviour is surprising especially in lambas where adding or removing the `mutable` specifier may lead to different behaviour. The problem isn't obvious and it's easy to wrongly drop the `mutable` druing a clean-up. Therefore introducing a tool for developers to fail at compile-time if that happens seems desired. Signed-off-by: Radoslaw Zarzynski --- src/crimson/common/utility.h | 16 ++++++++++++++++ .../seastore/onode_manager/staged-fltree/node.cc | 2 ++ 2 files changed, 18 insertions(+) create mode 100644 src/crimson/common/utility.h diff --git a/src/crimson/common/utility.h b/src/crimson/common/utility.h new file mode 100644 index 00000000000..42c199a959d --- /dev/null +++ b/src/crimson/common/utility.h @@ -0,0 +1,16 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:nil -*- +// vim: ts=8 sw=2 smarttab expandtab + +#pragma once + +#include + +template +void assert_moveable(T& t) { + // It's fine +} +template +void assert_moveable(const T& t) { + static_assert(always_false::value, "unable to move-out from T"); +} + diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc index cf585ff5334..db68ffb4f51 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc @@ -8,6 +8,7 @@ #include #include "common/likely.h" +#include "crimson/common/utility.h" #include "crimson/os/seastore/logging.h" #include "node_extent_manager.h" @@ -1828,6 +1829,7 @@ LeafNode::erase(context_t c, const search_position_t& pos, bool get_next) } return seastar::now().then( [c, &pos, this_ref = std::move(this_ref), this, FNAME] () mutable { + assert_moveable(this_ref); #ifndef NDEBUG assert(!impl->is_keys_empty()); if (impl->has_single_value()) { -- 2.39.5