Skip to content

Commit d3b0b8d

Browse files
fix(expression): disable the move constructor for public use to avoid undefined behaviour
1 parent a90573c commit d3b0b8d

File tree

1 file changed

+60
-13
lines changed

1 file changed

+60
-13
lines changed

include/boost/numeric/ublas/tensor/expression.hpp

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,30 @@ struct tensor_expression
123123
inline
124124
constexpr auto const& operator()() const noexcept { return *static_cast<const expression_type*> (this); }
125125

126-
constexpr tensor_expression(tensor_expression&&) noexcept = default;
127-
constexpr tensor_expression& operator=(tensor_expression&&) noexcept = default;
128126
constexpr ~tensor_expression() = default;
129127

128+
/// @brief The copy constructor is deleted to avoid expensive copies or sometimes object slicing.
130129
tensor_expression(const tensor_expression&) = delete;
131130
tensor_expression& operator=(const tensor_expression&) = delete;
131+
constexpr tensor_expression& operator=(tensor_expression&&) noexcept = delete;
132132

133-
133+
/**
134+
* @brief This is the only way to discourage the users from using `std::move` on the local
135+
* expression because it works differently from the standard way to move the objects. This weird
136+
* behaviour is due to `const reference`, which is impossible to move without constructing a new object.
137+
* If the variable goes out of the scope, stored as a `const reference` inside the expression,
138+
* it will be destroyed that will result in a dangling pointer. But this behaviour is helpful
139+
* for the construction of an expression because the expression might contain a `const reference`
140+
* object that will be passed around as a `const reference` rather than a copy, and we do not need to
141+
* construct a whole new chunky object because, under the hood, we are just passing pointers around.
142+
*
143+
*/
134144
protected :
145+
constexpr tensor_expression(tensor_expression&&) noexcept = default;
135146
explicit tensor_expression() = default;
147+
148+
/// @brief This the only way to access the protected move constructor of other expressions.
149+
template<class, class> friend struct tensor_expression;
136150
};
137151

138152

@@ -157,20 +171,36 @@ struct binary_tensor_expression
157171
, er(std::forward<RightExp>(r))
158172
, op(std::forward<OPType>(o))
159173
{}
160-
constexpr binary_tensor_expression(binary_tensor_expression&& l) noexcept = default;
161-
constexpr binary_tensor_expression& operator=(binary_tensor_expression&& l) noexcept = default;
162174
constexpr ~binary_tensor_expression() = default;
163175

176+
/// @brief The copy constructor is deleted to avoid expensive copies or sometimes object slicing.
164177
binary_tensor_expression(const binary_tensor_expression& l) = delete;
165178
binary_tensor_expression& operator=(binary_tensor_expression const& l) noexcept = delete;
179+
constexpr binary_tensor_expression& operator=(binary_tensor_expression&& l) noexcept = delete;
166180

167181
[[nodiscard]] constexpr auto const& left_expr() const noexcept{ return cast_tensor_expression(el); }
168182
[[nodiscard]] constexpr auto const& right_expr() const noexcept{ return cast_tensor_expression(er); }
169183

170184
[[nodiscard]] inline
171-
constexpr decltype(auto) operator()(size_type i) const {
172-
return op(left_expr()(i), right_expr()(i));
173-
}
185+
constexpr decltype(auto) operator()(size_type i) const { return op(left_expr()(i), right_expr()(i)); }
186+
187+
/**
188+
* @brief This is the only way to discourage the users from using `std::move` on the local
189+
* expression because it works differently from the standard way to move the objects. This weird
190+
* behaviour is due to `const reference`, which is impossible to move without constructing a new object.
191+
* If the variable goes out of the scope, stored as a `const reference` inside the expression,
192+
* it will be destroyed that will result in a dangling pointer. But this behaviour is helpful
193+
* for the construction of an expression because the expression might contain a `const reference`
194+
* object that will be passed around as a `const reference` rather than a copy, and we do not need to
195+
* construct a whole new chunky object because, under the hood, we are just passing pointers around.
196+
*
197+
*/
198+
protected:
199+
constexpr binary_tensor_expression(binary_tensor_expression&& l) noexcept = default;
200+
201+
/// @brief This the only way to access the protected move constructor of other expressions.
202+
template<class, class, class> friend struct unary_tensor_expression;
203+
template<class, class, class, class> friend struct binary_tensor_expression;
174204

175205
private:
176206
expression_type_left el;
@@ -213,20 +243,37 @@ struct unary_tensor_expression
213243
: e(std::forward<Exp>(ee))
214244
, op(std::forward<OPType>(o))
215245
{}
216-
constexpr unary_tensor_expression(unary_tensor_expression&& l) noexcept = default;
217-
constexpr unary_tensor_expression& operator=(unary_tensor_expression&& l) noexcept = default;
218246
constexpr ~unary_tensor_expression() = default;
219247

220248
constexpr unary_tensor_expression() = delete;
249+
250+
/// @brief The copy constructor is deleted to avoid expensive copies or sometimes object slicing.
221251
unary_tensor_expression(unary_tensor_expression const& l) = delete;
222252
unary_tensor_expression& operator=(unary_tensor_expression const& l) noexcept = delete;
253+
constexpr unary_tensor_expression& operator=(unary_tensor_expression&& l) noexcept = delete;
223254

224255
[[nodiscard]] constexpr auto const& expr() const noexcept{ return cast_tensor_expression(e); }
225256

226257
[[nodiscard]] inline constexpr
227-
decltype(auto) operator()(size_type i) const {
228-
return op(expr()(i));
229-
}
258+
decltype(auto) operator()(size_type i) const { return op(expr()(i)); }
259+
260+
/**
261+
* @brief This is the only way to discourage the users from using `std::move` on the local
262+
* expression because it works differently from the standard way to move the objects. This weird
263+
* behaviour is due to `const reference`, which is impossible to move without constructing a new object.
264+
* If the variable goes out of the scope, stored as a `const reference` inside the expression,
265+
* it will be destroyed that will result in a dangling pointer. But this behaviour is helpful
266+
* for the construction of an expression because the expression might contain a `const reference`
267+
* object that will be passed around as a `const reference` rather than a copy, and we do not need to
268+
* construct a whole new chunky object because, under the hood, we are just passing pointers around.
269+
*
270+
*/
271+
protected:
272+
constexpr unary_tensor_expression(unary_tensor_expression&& l) noexcept = default;
273+
274+
/// @brief This the only way to access the protected move constructor of other expressions.
275+
template<class, class, class> friend struct unary_tensor_expression;
276+
template<class, class, class, class> friend struct binary_tensor_expression;
230277

231278
private:
232279
expression_type e;

0 commit comments

Comments
 (0)