6

I have this line of code

//std::unique_ptr<SDL_Window> _window_; // this is somewhere else...
_window_ = std::make_unique<SDL_Window>(SDL_CreateWindow("SDL Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, _WIDTH_, _HEIGHT_, SDL_WINDOW_SHOWN));

it produces the following compiler error

In file included from /usr/include/c++/6/memory:81:0,
                 from /home/user/prj/src/main.cpp:4:
/usr/include/c++/6/bits/unique_ptr.h: In instantiation of ‘typename 
std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = SDL_Window; _Args = {SDL_Window*}; typename 
std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<SDL_Window>]’:
/home/user/prj/src/main.cpp:36:170:   required from here
/usr/include/c++/6/bits/unique_ptr.h:791:30: error: invalid use of incomplete type ‘struct SDL_Window’
     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }

why? (It works fine without smart pointers, so my guess is I didn't understand the syntax and this is trivial to fix. Will add source and CMakeLists.txt below.)

CMakeLists.txt

cmake_minimum_required(VERSION 3.7)
project(prj)

find_package(SDL2 REQUIRED)
include_directories(prj ${SDL2_INCLUDE_DIRS})

add_executable(prj main.cpp)
target_link_libraries(prj ${SDL2_LIBRARIES})

main.cpp

#include "SDL.h"
#include <memory>
#include <iostream>
#include <fstream>
#include <cstdint>

class Window
{

    public:

    Window()
        : _window_{nullptr}
        , _surface_{nullptr}
    {
        if(SDL_Init(SDL_INIT_VIDEO) < 0)
        {
            std::cerr << SDL_GetError() << std::endl;
        }
        else
        {
            _window_ = std::make_unique<SDL_Window>(SDL_CreateWindow("SDL Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, _WIDTH_, _HEIGHT_, SDL_WINDOW_SHOWN));

            if(_window_ == nullptr)
            {
                std::cerr << SDL_GetError() << std::endl;
            }
            else
            {
                _surface_ = std::make_unique<SDL_Surface>(SDL_GetWindowSurface(_window_.get()));
                SDL_FillRect(_surface_.get(), nullptr, SDL_MapRGB(_surface_->format, 0xFF, 0xFF, 0xFF));
                SDL_UpdateWindowSurface(_window_.get());
                SDL_Delay(1000);
            }
        }
    }

    ~Window()
    {
        SDL_DestroyWindow(_window_.get());
        SDL_Quit();
    }

    private:

    const int32_t _WIDTH_{600};
    const int32_t _HEIGHT_{400};

    std::unique_ptr<SDL_Window> _window_;
    std::unique_ptr<SDL_Surface> _surface_;

};


int main(int argc, char* argv[])
{
    Window window;
    return 0;
}
FreelanceConsultant
  • 13,167
  • 27
  • 115
  • 225

3 Answers3

13

Solution

Finally figured out the answer with a lot of trial and error, so will explain the solution here.

This is the correct syntax:

// first define the unique_ptr as member of class
std::unique_ptr<SDL_Window, decltype(&SDL_DestroyWindow)> _window_;

// second, initialize in the member initialization list of class constructor
// probably don't need to do this if not embedding as member of class
class_name()
    : _window_(nullptr, SDL_DestroyWindow)
{
    // blaa blaa SDL code etc
    _window_.reset(SDL_CreateWindow("SDL Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, WIDTH, HEIGHT, SDL_WINDOW_SHOWN));
}

// finally we need to be able to delete
// but this is handled automatically

Explanation

When we add the unique_ptr as a data member, we need to give both the type SDL_Window and the "deleter function format / syntax", becuase an ordinary delete call is not correct. We use decltype to automatically construct the correct deleter format from the deleter function. (Perhaps not the most accurate explanation.) In a way, decltype is somewhat like auto...

std::unique_ptr<SDL_Window, decltype(&SDL_DestroyWindow)> _window_;

This object must be initialized. We do this in the constructor. We set the pointer to nullptr (because we do not want to initialize it before initializing SDL2) and we also set the deleter function.

: _window_(nullptr, SDL_DestroyWindow)

After initializing SDL, we then want to create a window. This is done most easily by calling the smart pointer reset() function. We pass it a new pointer returned by the function which creates the window.

_window_.reset(SDL_CreateWindow(...));

Done. Took a long time to figure out but makes sense now. References

http://en.cppreference.com/w/cpp/memory/unique_ptr

Why does my unique_ptr think is has a null function pointer deleter?

What's wrong with this initialization of unique_ptr?

FreelanceConsultant
  • 13,167
  • 27
  • 115
  • 225
  • A custom deleter is what you want, but note that your code can be improved further: http://coliru.stacked-crooked.com/a/adc9e7f6bf45e0ca – milleniumbug Feb 07 '18 at 21:21
  • @milleniumbug How? That doesn't look like an improvement to me – FreelanceConsultant Feb 07 '18 at 21:23
  • Refer to the link. Using a stateless function object reduces the size of the smart pointer, and you don't need to pass in the function to the constructor of `std::unique_ptr` – milleniumbug Feb 07 '18 at 21:24
  • Note that you don't need the constructor initializer. You can inline-initialize `_window_` directly: `std::unique_ptr _window_{nullptr, SDL_DestroyWindow};` – Nikos C. Feb 07 '18 at 21:29
  • @milleniumbug Apologies. I missed the point here (size reduction.) – Nikos C. Feb 07 '18 at 21:41
  • @NikosC. No I can't due to the nature of how SDL works - if the same class calls `SDL_Init` then this won't work. – FreelanceConsultant Feb 07 '18 at 21:42
  • You're initializing to `nullptr`, so it will be fine. (Btw, I'm only refering to the member initializer list, not the `.reset()` call in the ctor body. That stays as-is, obviously.) – Nikos C. Feb 07 '18 at 21:45
  • Oh, btw, in case it wasn't obvious, this is simply based on the fact that `unique_ptr` will not actually call the deleter if the managed pointer is null. – Nikos C. Feb 07 '18 at 21:47
  • @NikosC. Oh, right I've just realized what you mean - thanks – FreelanceConsultant Feb 07 '18 at 21:51
2

Those structures are opaque data structures, you don't have the full definition of them. That means, among other things, that the default std::unique_ptr deleter don't know how to "delete" the structures.

You need to provide your own custom deleter. For SDL_Window it's the SDL_DestroyWindow function.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

A SDL Windows is destroyed using SDL_DestroyWindow, not plain delete, like the unique_ptr's default deleter does. You need to supply a custom deleter to unique_ptr, which will call SDL_DestroyWindow.

The Techel
  • 918
  • 8
  • 13