Pull to refresh

Comments 49

Фактически, вы изобрели вариант, вроде boost::variant. А использованная конструкция называется tagged union. Для упрощения создания таких штук в C++11 предусмотрели std::aligned_union (кстати, вопрос выравнивания у вас не затронут).
Ну и вообще, в C++11 есть relaxed unions: разрешено иметь объединения, содержащие любые типы, но необходимо в ручную вызывать placement new и деструктор (собственно, вы такое объединение объявили, но использовали почему-то только для вычисления размера, но не напрямую для хранения элементов). Собственно, массив mem не нужен, а достаточно поместить объединение в класс. Почитать
В этом случае как раз и получится то, что у меня представлено во втором варианте. Дело в том, что 1) доступ к членам union осуществляется по их именам, а не типам, и 2) мы не сможем создать объект такого объединения, если он будет содержать более одного не POD типа с нетривиальным конструктором. Попробуйте сами.
У меня тоже компилируется. Я использую clang 3.2-7 и gcc 4.8.1. Ваш пример с Variant я посмотрю. Пока просто не успел. Обязательно разберусь с этим подходом.
А на счёт доступа по именам — это решается с помощью рекурсивного объединения. По ссылке, которую я привел, описан весь процесс создания Variant таким методом.
Сделал перевод статьи и оставляю этот комментарий в качестве перекрёстной ссылки для заинтересовавшихся.
Круто! Вы успели перевести быстрее чем я прочитать.
Спасибо за интересные ссылки.
По поводу выравнивания. Так ведь sizeof возвращает размер с учетом выравнивания. Так что не вижу с этим проблемы. Или какой то еще подвох здесь есть?
На счет std::aligned_union согласен, вместо unsigned char mem[usize] лучше использовать его.
sizeof возвращает размер с учетом выравнивания после объекта.
А для корректной работы может понадобиться выравнивать начало размещения объекта в памяти по границе, которая не совпадает со стандартным выравниванием начала массивов char.
Ага понял. Тогда да, — это грабли. Ладно, вечерком посмотрю, подправлю.
Фактически, вы изобрели вариант, вроде boost::variant.


С boost::variant код выглядел бы следующим образом и имел бы следующие плюсы:
class BaseCreator
{
    // boost::none_t - чтобы иметь пустое состояние и конструктор по умолчанию без накладных расходов
    typedef boost::variant<boost::none_t, A, B> obj_t;
    obj_t obj;

public:
    BaseCreator() = default; // noexcept

    BaseCreator(bool x)
        : obj(x ? obj_t(A()) : obj_t(B()))
    {}

    // Можно не запрещать копирование :)
    BaseCreator(const BaseCreator &) = default;
    // Перемещение
    BaseCreator(BaseCreator &&) = default;

    Base* operator->()
    {
        // polymorphic_get был добавлен в Boost 1.56. Рекомендую к употребелению! Классная штука.
        //
        // Если это единственное место где используется Base, то 
        // деструктор можно не делать виртуальным.
        //
        // Кстати, Ваш пример будет некорректно работать в случае сложного множественного наследования:
        // так reinterpret_cast<T*>(mem); не справится с struct A: virtual Base1, virtual Base {...}
        return &boost::polymorphic_get<Base>(obj);
    }
};

Код примера и с обычным множественным наследованием работать не будет, достаточно, чтобы адрес базового класса не совпадал с адресом начала объекта.
Да, получилось весьма симпатично.
Какая-то странная фабрика. Может вернуть только один объект за раз. И конструктор вызовется непонятно когда. Вызывающий не может детерминировано удалить его. Почему вообще не сделать пул, если уж на то пошло?
Не представляю, где его можно на практике применить. Может приведете пример?
Может вернуть только один объект за раз

Потому что это не «фабрика», а «фабричный метод». Собственно переделать всю конструкцию в фабрику по общему принципу тоже можно. Просто на примере фабричного метода проще проиллюстрировать подход.
Деструктор явно вызывать не нужно, т.к.стековые объекты существуют лишь в границах содержащего их блока. Собственно с классическим фабричным методом тоже самое, если он возвращает не сырой указатель, а, как положено, unique_ptr.
Что касается применения, то оно ни чем не отличается от применения обычного фабричного метода с динамическим размещением. См. первый попавшийся пример из гугла
Деструктор вызвать все-таки нужно. Не нужно освобождать память.
Так он и вызывается автоматически. Во всех примерах. Гляньте еще разок.
Деструктор нельзя не вызывать. Может там ресурсы какие-то освобождаются.

>Потому что это не «фабрика», а «фабричный метод».
Ну и что? Creator может несколько объектов запросить.
Да потому что это не фабричный метод, а этакой pimpl
Очень удивился, увидев non-POD типы в union, но потом сообразил, что видимо речь идет про С++11.
То есть std::unique_ptr, override и -std=c++11 вас не смутили в первом же сниппете? :)
У вас большие проблемы с выравниванием и копированием/переносом.

Нельзя просто взять и скопировать объект в C++, скопировав его данные. Нужно еще вызвать его конструктор копирования.

И вообще нужно просто использовать small obejct allocator-ы, и не париться.
Копирование запрещено. На счет конструктора перемещения, похоже, вы правы. С TypeUnion(TypeUnion &&) = default я поторопился. Еще одни грабли.
Сейчас еще раз подумал. Мы же добиваемся такого же поведения, как и при использовании динамического выделения. Так что, запрещенный конструктор копирования и дефолтный конструктор переноса — то что надо. Собственно unique_ptr так и работает.
Таким образом, с копированием/переносом проблем нет.
Не добиваемся мы. Для того, чтобы добиться этого, надо чтобы объект не перемешался при перемещении `BaseCreator`-а. `unique_ptr` работает именно _так_, а ваш код — не так.
Да, каюсь. Глупость ляпнул.
Дефолтный конструктор перемещения BaseCreator просто вызывает конструктор перемещения TypeUnion, и это адекватное поведение. Но вот конструктор перемещения TypeUnion, вместо того чтобы вызвать конструктор перемещения того объекта, который в нем в данный момент хранится, тоже объявлен дефолтным, а это значит, что он просто копирует (через memcpy) внутренний буфер и указатель. Если в TypeUnion будет хранится, например, unique_ptr, то это приведет к тому, что у вас станет два unique_ptr'а, владеющие одной и той же памятью, в результате чего, например, после вызова деструкторов обоих TypeUnion один и тот же участок памяти будет освобожден дважды.
Интересный факт, что ваш код (который под спойлером "Полный код примера") крашится во время работы, если скомпилировать его на GCC 4.9.0 c флагом -O2: coliru.stacked-crooked.com/a/1742a8366403abef Если компилировать clang++, GCC 4.8.2 или понизить флаг до -O1, то все работает как надо. Не готов сказать баг ли это компилятора, или у вас какое-то хитрое UB в коде, проблема в вызове p->f() и она как-то связана с инлайном, потому что с ключами -O2 -fno-inline все начинает работать.
Я думаю UB более вероятно :)
На моих clang 3.2-7 и gcc 4.8.1 я воспроизвести не могу. Но по вашей ссылке видно, что выход происходит в строке 72
   assert ( deleter ); // TypeUnion::assign was not called
Т.е. каким то образом deleter == nullptr. Попробую найти GCC 4.9.0 и отдебажить.
Еще чуть выше мне подсказали о возможных проблемах с выравниванием. Может как то с этим связано. Короче, надо будет разобраться.
assert там не виноват, если его закомментировать, все равно будет падать. Выравнивание я поправлял, не помогло.
Ну да assert просто сигнализирует, что не был вызван assign. Который инстацирует объект. Если объект не инстацирован, то падать оно конечно будет т.к. в памяти не то что мы ожидаем. Просто такого там быть не должно. Но в g++-4.9.2 проблема не воспроизводится.
Только что прогнал по g++-4.9.2 c разными флагами. Все ок. Никаких проблем.
Странно это, извините за беспокойство. Просто хотел кое-что проверить, закинул ваш пример в онлайн-компилятор, а там такое вот…
Да нет, все нормально. Мне самому интересно. Сижу вот до сих пор дебажу прямо по вашей ссылке. Надо же выяснить в чем там дело.
Если все же найдете проблему, не забудьте поделиться с общественностью.
Видимо, все дело тут в некорректном reinterpret_cast в функции get, который приводит к UB, как сообщают нам antoshkka и lemelisk чуть выше
Он некорректен только в случае множественного наследования, то есть когда у одного класса сразу несколько базовых. У вас в примере таких классов нет. С множественным наследованием проблема в том, что когда у объекта несколько базовых классов, то адрес одного из них будет не совпадать с адресом начала объекта, и для корректного преобразования нужно будет не только поменять тип указателя, а ещё и сдвинуть его на некоторую величину.
Вы сохранили B*, а прочитали Base* — это UB, по определению. А в нормальных программах не бывает UB. А раз тут нет UB, значит этот код никогда не выполнится. А это возможно только тогда, когда assert всегда срабатывает. А раз assert всегда срабатывает, то и не фиг его проверять. Именно так и поступил gcc, там __assertion_failed вызывается безусловно. Можете посмотреть вывод ассембелра.
А почему это UB? Ведь Base — базовый класс B. Если заменить obj.get<Base>(); на obj.get<B>(); от краша все равно не спасает.
Хм. Возможно, бага компилятора в этом месте. Потому что замена , return reinterpet_cast<...> на return new B() таки от креша спасет, так что я уверен, что логика его именно в том, что reinterpet_cast приводит к UB.
Обязательно. Но вчера я так и не нашел причину. Сложность еще и в том, что «дебажить» приходится в браузере, что весьма необычно для меня. (у меня нет g++-4.9.0, а только g++-4.9.2 и g++-4.8.1)
Нашел баг!!! coliru.stacked-crooked.com/a/d919304815452d35
Все таки не зря encyclopedist с самого начала сказал о проблемах с выравниванием, а я вместо того что бы устранить грабли принялся дебажить изначально дефективный код.
Если кратко, как исправлен баг:
Добавлено
template <std::size_t Len, typename ...Types>
struct aligned_union {
    static constexpr size_t max(size_t r0) {return r0;}
    template <typename ...R> static constexpr size_t max(size_t r0, R... r) {return ( r0 > max(r...) ? r0 : max(r...) );}
    static constexpr std::size_t alignment_value = max(alignof(Types)...);
    struct type { alignas(alignment_value) unsigned char _[max(Len, sizeof(Types)...)]; };
};

т.к. в этой версии gcc, видимо, std::aligned_union уже удален.
    unsigned char mem[usize];
заменено на
    typedef typename aligned_union<usize, Types...>::type aligned_type;
    aligned_type _storage;
    aligned_type *mem = &_storage;

Таким образом, проблема решилась добавлением выравнивания.

В моем случае можно было поступить и проще. Просто заменить unsigned char mem[usize]; на
alignas(max(alignof(Types)...)) unsigned char mem[usize];
Нет, дело не в выравнивании (я его исправлял, не помогает), вы добавили лишний член — указатель mem, просто чтобы хранить в нем адрес другого члена этого же объекта (_storage), и магическим образом это помогло. Замените в функции get return static_cast<T*>(static_cast<void*>(mem)); на return static_cast<T*>(static_cast<void*>(&_storage)); и снова начнет крашится.
Не, это не считается. Вы слишком сильно переписали код.

Если заменить ваш unsigned char mem[usize]; на alignas(max(alignof(Types)...)) unsigned char mem[usize];, то делу это не поможет, все равно будет падать. Так что проблема не в alignment-е. Более того, если написать
unsigned char mem_s[usize];
void* mem = mem_s;

то этого будет достаточно, чтобы компилятор не включал дуркуоптимизацию. Но дело в том, что если компилятор шалит, то либо у вас UB, либо в компиляторе ошибка. Когда вы так сильно переписали код, то в первом случае вы могли только спрятать UB, и поэтому это не может считаться решением пока вы не ответили, что конкретно в вашем коде было не так.
aligned_union не удален, он наоборот, ещё не добавлен. Он будет добавлен только в GCC версии 5. (Я при написании первого комментария, честно говоря, подзабыл о том, что оно ещё недоступно в GCC, хотя сам несколько мес. назад сталкивался с этим)
Хранение объектов в «куче» — накладные расходы. Если хочется объекты переменной длины размещать в стеке, то можно ещё использовать такую технику: «Размещение объектов переменной длины с использованием двух стеков», «Реализация двухстековой модели размещения данных». Правда, она требует ассемблерных вставок.
Насколько я понимаю, никакого «особого, аппаратного стека» не существует. Просто call, ret, enter, leave, push, pop неявно используют ebp и esp. Поэтому и ассемблерных вставок в общем-то не надо, чтобы иметь глобальный динамический стек под объекты.

Как самый простой вариант: alloca() — неявно выделяет память прямо на «родном» esp-стеке. Освобождает при выходе из функции. Деструкторы можно навесить, завернув такие объекты в умный указатель. Однако, alloca() considered harmful, потому что обычно у стека весьма скромный объём. Поэтому чуть более сложный вариант: засунуть в thread-local storage указатель на кусок глобальной памяти под собственный стек и написать свою alloca(), которая выделяет память оттуда.
Sign up to leave a comment.

Articles