VasilyK Sep 26 2016 at 15:44

Умный указатель для Pimpl

6 min

15K

C++*

From sandbox

+20

Comments 10

Door Sep 26 2016 at 17:07

Я бы всё-же не приводил "не очень хороший" код в статье для новичков: в первом примере, раз вы явно говорите об деструкторе, то можно бы и явно прописать реализацию конструкторов/операторов присваивания, следуя хорошему тону — The rule of three/five/zero. auto_ptr я бы выбросил вообще — всё по тому же поводу — deprecated — можно просто ссылочку дать — так для истории и действительно интересующихся. Про unique_ptr, возможно, нужно было бы рассказать, почему деструктор нужно реализовывать в файле-реализации (а что с move-семантикой на этот счёт ?) — но это отклонение от темы — зачем нужна своя реализация PimplPtr-а?

Если убрать из статью вводную, то причина у вас, получается одна — "нарушение логической константности" (а точнее 2ве: "propagate_const пока не является частью стандарта"). Хорошо, но — код из статьи — не компилируется.

По поводу реализации:

static_assert — не нужен — это сделает за вас вызов конструктора в make_unique.
constexpr — зачем? Он сдесь только мешает код читать и нет случая, когда он нужен был бы.
2й конструктор, который explicit (кстати, а почему? почему и не первый ?), наверное, должен вызывать первый (Delegating constructor) — хотя бы для того, чтобы логика в конструкторах совпадала
зачем в деструкторе делать тот-же statis_assert, когда в конструкторе — она уже есть ?
почему здесь = default используется, а в примерах выше, в файле-реализации — нет ?: Widget::~Widget() = default
хорошим тоном является использование одной и той же функции — основной — для реализации других ф-й, которые дублируют код для удобства, т.е. — реализация операторов должна использовать эталонный get()

Спасибо за статью. Извините за, возможно, резкий тон. Хорошего вам дня ^_^

VasilyK Sep 26 2016 at 18:34

По поводу "не очень хорошего" кода, я с вами согласен, так как это имеет прямое отношение к рассматриваемому вопросу. Можно было показать в чем заключается проблема, и то, что unique_ptr по умолчанию запрещает попасть в неприятную ситуацию. И тут же описать использование move семантики.

Рассказать, почему деструктор нужно реализовывать в файле-реализации я думаю не стоит, так как это не совсем относиться к рассматриваемой теме, одна ссылка для ознакомления приведена в статье.

“не компилируется” — поправил.

“то причина у вас, получается одна” — еще как минимум одна причина — конструктор по умолчанию, я часто забывал написать явное создание Impl, после чего несколько минут искал ошибку.

По поводу реализации:

static_assert действительно можно опустить, но я его использую для более приятного сообщения об ошибке.
constexpr — пожалуй, вы правы.
Если вызывать конструктор из конструкторы, то можно было бы из первого вызывать второй (но не наоборот), но не думаю что это было бы лучше. ?? explicit
static_assert нужен как в конструкторе так и деструкторе — сообщения об ошибке отличаются, они указывают предполагаемую причину ошибку, если забыли конструктор, ты будет первое сообщение, если забыли деструктор — второе.
возможно "= default" было бы лучше
Согласен с вами, исправил исходный код.

Спасибо за комментарий.

Door Sep 26 2016 at 20:59

Спасибо за ответ.

если забыли конструктор, ты будет первое сообщение, если забыли деструктор — второе.

Я немного не понял вас. Вот моя логика: sizeof(T) — приведёт к ошибке компиляции, если компилятор не видет определения типа T, т.е., T — неполный тип. sizeof(T) никак не зависит от того определён для user defined типа конструктор или деструктор. Это означает, что sizeof(T) > 0 всегда — для нашого случая — нужен только для того, чтобы выдать пользователю ошибку во время компиляции о том, что он забыл определить указанный класс (тип) до места его использования (кстати, поскольку sizeof(T) никогда не может быть 0м — то можно просто писать sizeof(T)).
Дальше: у вас, по сути, таких места 2: конструктор и деструктор. Ставим вопрос — может ли пользователь написать такое использование PimplPtr<Impl>, чтобы Impl был, например, определён до вызова конструктора PimplPtr<Impl> и, одновременно, не определён при вызове деструктора PimplPtr<Impl>? (или наоборот). Ответ — да, может:

Случай первый: в конструкторе тип - неопределён, в деструкторе - уже определён

    template<typename T>
    struct PimplPtr
    {
        // 
    };

    // Header
    // 
    struct UserType
    {
        struct Impl;
        PimplPtr<Impl> _impl;

        UserType()
            // Используем неполный тип `Impl`
            : _impl{}
        {
        }

        ~UserType();
    };

    // Source
    // 

    struct UserType::Impl
    {
    };

    // Используем уже определённый тип `Impl`
    UserType::~UserType() = default;

Случай второй: в конструкторе тип - определён, в деструкторе - не определён

    // Header
    // 
    struct UserType
    {
        struct Impl;
        PimplPtr<Impl> _impl;

        UserType();

        ~UserType()
        {
            // Используем неполный тип `Impl`
        }
    };

    // Source
    // 

    struct UserType::Impl
    {
    };

    // Используем уже определённый тип `Impl`
    UserType::UserType()
        : _impl{}
    {
    }

Т.е., да — я вас обманул в деструкторе — для приведённого второго случая — такой assert будет полезен, прошу прощения. Но текст сообщения, всё же, немного неправильный.

По поводу делегирующего конструктора — я снова вас обманул — имел в виду, что 1й конструктор должен вызывать 2й, т.е., для вашего кода, это что-то типа:

PimplPtr(): PimplPtr(std::make_unique<T>()) {}

explicit PimplPtr(std::unique_ptr<T>&& p) noexcept: p_(std::move(p)) {}

И ещё — поскольку у вас в публичном классе есть функция:

PimplPtr(std::unique_ptr<T>&& p)

То это означает, что я могу сделать так:

PimplPtr<int>{std::unique_ptr<int>{nullptr}};

либо, более неявно — через какую-то фабрику:

PimplPtr<T>{make_T()};

где make_T() вернёт нулевой указатель.
Т.е. не хватает проверки времени выполнения инварианта указатель не nullptr — я бы добавил assert() в конструктор и ф-и get().

И, моё предвзятое мнение — в мире плюсов — для таких штук как умные указатели — пытаются убежать от неявных опеаторов преобразования: т.е. — пометить бы ещё operator ElementType() как explicit...

И… и я бы написал using ElementType = T — меньше кода и проще читается…
Извините, я увлёкся — простите за, возможно, нелепую критику — ухожу от клавиатуры :)
Спасибо

VasilyK Sep 26 2016 at 21:34

"Но текст сообщения, всё же, немного неправильный" — что вы имеете ввиду? Я рассматриваю это сообщение как небольшую подсказку, чтобы быстрее вспомнить что может быть не так. Если человек не понимает о чем речь, то ему наверняка придется идти на какой-то ресурс и искать более подробную информацию.

"Т.е. не хватает проверки времени выполнения инварианта указатель не nullptr" — полностью согласен

"operator ElementType() как explicit..." — скорее всего это правильно, но в данном случае возможно это не важно

dix75 Oct 22 2016 at 20:43

Не соглашусь с auto_ptr, не все мы живем в идеальном мире, приходится пользоваться и старыми компиляторами.
Переход на новые не всегда возможно, иногда даже вообще невозможно

VasilyK Sep 26 2016 at 18:32

del

-1

VitaminPSG Sep 26 2016 at 21:16

1) Не стоит объявлять пустые деструктор в классах. Т.к. вы вводите запрет на создание конструкторов и операторов присваивания по умолчанию. А в. будущем и на копирования.
2) Константные методы можно реализовать в интерфейсе.

VasilyK Sep 26 2016 at 21:27

Не совсем понял про деструкторы. Если вы говорите про класс widget, то деструктор необходим для нормальной работы Pimpl, если же говорите про класс PimplPtr, то, я думаю, нет никаких проблем явно написать конструкторы и операторы присваивания.

Второе совсем не понял, константные методы где? В классе widget? Зачем их реализовывать через интерфейс?

Adamantium Sep 27 2016 at 08:49

Спасибо, интересно изложено, и местами стало понятнее. Ещё и по ссылкам про внутренности Qt, стало ещё понятнее.
А может подскажете, в каком направлении искать/читать, как принято реализовывать модульность, несколько подобную этой идиоме pimpl, но в случае, когда в рантайме необходимо выбирать, какая конкретно из разных реализаций одного интерфейса выбирается. Различные реализации для различных входных данных. Каждая реализация в своей dll/so с одинаковыми названиями функций. Я правильно понимаю, что подобная задача возникает в медиаплеерах, когда открывается файл, и в зависимости от его формата подключаются разные декодеры потока? Но сформулировать правильный вопрос гуглу не смог, к сожалению.

VasilyK Sep 27 2016 at 08:49

Если вы хотите выбирать реализацию в рантайме, то, скорее всего, вам нужна некоторая фабрика и, возможно, шаблон проектирования мост.

Класть каждую реализацию в отдельную динамической библиотеку мне не приходилось, но, как я понимаю, система плагинов QtCreator устроена подобным образом, так что можете посмотреть в эту сторону.

“Я правильно понимаю, что подобная задача возникает в медиаплеерах” — возможно, не сталкивался.

Show the best of all time