Passage d'un unique_ptr à une méthode

Bonsoir,

Je pensais avoir compris à l’aide de ce post sur StackOverflow comment passer un unique_ptr à une méthode. Or apparemment non. J’ai créé deux classes pour m’y essayer : Song et Library. La deuxième a un attribut qui est un vector de la première.

1
2
3
4
5
6
7

Song::Song(const std::string &title, const std::string &artist) : title(title), artist(artist)
{}

void Song::print() const
{
    std::cout << getTitle() << " by " << getArtist() << std::endl;
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

class Library
{
public:
    Library();
    void addSong(std::unique_ptr<Song> &song);
    std::vector<std::unique_ptr<Song>> get() const;

private:
    std::vector<std::unique_ptr<Song>> library = std::vector<std::unique_ptr<Song>>();
};

Library::Library()
{}

void Library::addSong(std::unique_ptr<Song> &song)
{
    library.push_back(std::move(song));
}

std::vector<std::unique_ptr<Song>> Library::get() const
{
    return library;
}

Voici ce que je fais dans mon main :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12

int main()
{
    auto library = Library();

    auto song = std::make_unique<Song>("Sunset Lover", "Petit Biscuit");
    song->print();

    library.addSong(song);
    library.get()[0]->print();

    return 0;
}

J’ai le droit à la fameuse erreur :

1

error: use of deleted function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = Song; _Dp = std::default_delete<Song>]'

Pourtant mon pointeur est passé en référence à ma méthode et ensuite déplacé avec std::move, il ne me semble pas en faire une copie à un seul moment ici.

Merci pour votre aide !

Salut,

je crois que la copie s’effectue sur le retour du get, puisque tu essaie de copier ton vecteur contenant les unique_ptr

en correction je propose la fonction const std::vector<std::unique_ptr<Song>> & get() const;

Mais sinon ça me paraît plus juste d’avoir la fonction

1
2
3
4

void addSong(std::unique_ptr<Song> && song)
{
    library.push_back(std::forward(song));
}

qui s’appelle par conséquent en explicitant le déplacement

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12

int main()
{
    auto library = Library();

    auto song = std::make_unique<Song>("Suset Lover", "Petit Biscuit");
    song->print();

    library.addSong(std::move(song));
    library.get()[0]->print();

    return 0;
}

évitant ainsi de futures confusions, puisque lors de l’appel de addSong, rien ne nous indique qu’on perd la variable avec ton implémentation

J’allais proposer la même solution que leroivi !

En fait, lorsque tu utilises void addSong(std::unique_ptr<Song> &song);, tu ne transferts pas la propriété de l’objet, donc il appartient toujours à ton "main". C’est comme si tu faisais juste un prêt le temps que la fonction "addSong" s’éxécute, après, tu rends le u_ptr à son propriétaire (parce que voler, c’est mal, cf. Pokémon).

L’utilisation de void addSong(std::unique_ptr<Song> &&song); va te permettre de transférer la propriété de ton u_ptr, il appartiendra dorénavant à ta librairie.

Edit: Je ne suis pas sûr que ça soit une bonne idée de faire changer de propriétaire (surtout si tu finis par le faire plusieurs fois), pourquoi ne fais tu pas une fonction qui instancie directement dans ta librairie ?

Merci pour votre aide !

J’ai toujours un message d’erreur après les modifications :

1
2

In member function 'void Library::addSong(std::unique_ptr<Song>&&)':
error: no matching function for call to 'forward(std::unique_ptr<Song>&)

EDIT : corrigé :

1

library.push_back(std::forward<std::unique_ptr<Song>>(song));

J’essaye également de comprendre les modifications. D’après ce que j’ai compris, && sert à référencer une valeur qui n’a pas d’adresse mémoire (rvalue), comme 5 ou Song(). Pourquoi la propriété est transférée ?

const std::vector<std::unique_ptr<Song>> & get() const; me permet d’obtenir une référence vers mon attribut et non une copie (ce qui permet de ne pas copier les u_ptr) c’est ça ?

Edit: Je ne suis pas sûr que ça soit une bonne idée de faire changer de propriétaire (surtout si tu finis par le faire plusieurs fois), pourquoi ne fais tu pas une fonction qui instancie directement dans ta librairie ?

Ce code n’a aucun intérêt, j’essaye de comprendre les u_ptr. Mais sinon oui, ce serait plus intelligent !

En fait, c’est le move qui fait le transfert de propriété, && sert à dire que la valeur est "aspirée" (qu’elle ne devrait plus être utilisé ici dans "main" après l’appel).

D’accord, et quel est le but de std::forward ?

Je dirais de retransformer song (devenu une lvalue) en rvalue;

C’est quand même assez tordu

Merci pour votre aide et bonne soirée!

Au passage, std::forward est pas vraiment prévu pour transférer la propriété (ça le fait, mais c’est pas l’utilisation principale). Il est mieux d’utiliser simplement std::move de nouveau, ce qui permet aussi d’éviter d’indiquer le type de l’objet à transférer (ce qui est nécessaire pour std::forward).

???

Copie! C’est fait pour être copié les unique_ptr – enfin…, "fait pour être passé par copie qui demande en fait un mouvement". Cela te simplifiera grandement le code utilisateur.

Aussi

1

    std::vector<std::unique_ptr<Song>> library = std::vector<std::unique_ptr<Song>>();

est inutilement compliqué. Le constructeur par défaut fait déjà ça.

D’ailleurs, le constructeur par défaut généré par défaut fait déjà ce qu’il faut.

Enfin, retourner une copie du vecteur interne est une mauvaise idée à cause des pointeurs uniques. Au pire, revoie une référence constante, sinon, mieux fournis des services de type vecteur au niveau de classe qui renvoient des références vers le type des éléments. Fait une vraie couche d’encapsulation au niveau de la classe utilisatrice.