From 002a6da1a1d5268153a9cda13edacc09fc135238 Mon Sep 17 00:00:00 2001 From: Minijackson Date: Fri, 9 Mar 2018 11:31:31 +0100 Subject: Add License header + elaborate on the placement new --- framework/src/errors.h | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/framework/src/errors.h b/framework/src/errors.h index ec5bed7b..d95272da 100644 --- a/framework/src/errors.h +++ b/framework/src/errors.h @@ -1,3 +1,21 @@ +/* + Copyright (c) 2018 Christian Mollekopf + + This library is free software; you can redistribute it and/or modify it + under the terms of the GNU Library General Public License as published by + the Free Software Foundation; either version 2 of the License, or (at your + option) any later version. + + This library is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Library General Public + License for more details. + + You should have received a copy of the GNU Library General Public License + along with this library; see the file COPYING.LIB. If not, write to the + Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301, USA. +*/ #pragma once #include @@ -92,6 +110,19 @@ protected: { // This is a constructor, you have to construct object, not assign them // (hence the placement new) + // + // Here's the problem: + // + // Object that are part of a union are not initialized (which is + // normal). If we replaced the placement new by a line like this: + // + // ``` + // mValue = other.mValue; + // ``` + // + // If overloaded, this will call `mValue.operator=(other.mValue);`, but + // since we're in the constructor, mValue is not initialized. This can + // cause big issues if the Type is not trivially (move) assignable. if (mIsValue) { new (std::addressof(mValue)) Type(other.mValue); } else { @@ -101,8 +132,7 @@ protected: StorageBase(StorageBase &&other) : mIsValue(other.mIsValue) { - // This is a constructor, you have to construct object, not assign them - // (hence the placement new) + // If you're thinking WTF, see the comment in the copy constructor above. if (mIsValue) { new (std::addressof(mValue)) Type(std::move(other.mValue)); } else { -- cgit v1.2.3