Discussion :

[Invité]

Salut,

J'essaye de retourner un pointeur sur un objet issus d'un std::unique_ptr :

Code cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
Entity* CellMap::getEntityInside (unsigned int index) {
     if (index >= 0 && index < entityInside.size()) {
         return entityInside[index].get();
     }
     return nullptr;
}

mais le compilateur (Clang 3.5) me sort cette erreur : (Hors que je n'alloue aucun objet)

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unique_ptr.h|765|error: allocating an object of abstract class type 'odfaeg::graphic::Entity'|

[mintho carmo]

Ta classe Entity est abstraite et ne peut pas être allouée. Soit ce n'est pas ce code qui provoque cette erreur, soit tu utilises une allocation "cachée" (par exemple si entityInside est une std::map ? Mais dans ce cas, le if serait étrange)

HS : A mon sens, il serait mieux d'utiliser un assert plutôt qu'un if ici, ce qui permet de virer la contrainte "retourne nullptr si l'index n'est pas valide" et donc retourner une référence sur unique_ptr plutôt qu'un pointeur nu
HS2 : c'est inutile de tester si un unisgned est positif

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
unique_ptr<Entity>& CellMap::getEntityInside (unsigned int index) {
    assert (index < entityInside.size());
    return entityInside[index];
}
 
const unique_ptr<Entity>& CellMap::getEntityInside (unsigned int index) const {
    assert (index < entityInside.size());
    return entityInside[index];
}

[Invité]

Non entityInside est un vector, bref, je vais essayer de faire comme tu m'as dis.

[mintho carmo]

Donc l'erreur n'est pas dans le code donné. Montre la pile d'erreurs complète

[Invité]

Ok, la voici.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
 
||=== Build: Debug in ODFAEG (compiler: GNU GCC Compiler) ===|
||warning: optimization flag '-fexpensive-optimizations' is not supported|
||warning: argument unused during compilation: '-fexpensive-optimizations'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/fastDelegate.h|170|warning: 'RefVal' defined as a struct template here but previously declared as a class template [-Wmismatched-tags]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/fastDelegate.h|70|note: did you mean struct here?|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/resourceManager.h|528|warning: unused variable 'id' [-Wunused-variable]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|60|warning: 'odfaeg::physic::BoundingVolume::intersects' hides overloaded virtual functions [-Woverloaded-virtual]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|36|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingBox &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|37|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingSphere &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|38|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingEllipsoid &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|39|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::OrientedBoundingBox &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|40|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingPolyhedron &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/stateParameter.h|32|warning: field 'name' will be initialized after field 'value' [-Wreorder]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp|62|warning: comparison of unsigned expression >= 0 is always true [-Wtautological-compare]|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unique_ptr.h|765|error: allocating an object of abstract class type 'odfaeg::graphic::Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp:19:41: note||in instantiation of function template specialization 'std::make_unique<odfaeg::graphic::Entity, odfaeg::graphic::Entity *>' requested here|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|120|note: unimplemented pure virtual method 'operator==' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|188|note: unimplemented pure virtual method 'isAnimated' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|195|note: unimplemented pure virtual method 'isModel' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|201|note: unimplemented pure virtual method 'selectable' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|207|note: unimplemented pure virtual method 'isLight' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|213|note: unimplemented pure virtual method 'isShadow' in 'Entity'|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/entity.h|219|note: unimplemented pure virtual method 'isLeaf' in 'Entity'|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_construct.h|75|error: call to deleted constructor of 'std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >'|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:75:8: note||in instantiation of function template specialization 'std::_Construct<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > &>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:125:2: note||in instantiation of function template specialization 'std::__uninitialized_copy<false>::__uninit_copy<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:278:19: note||in instantiation of function template specialization 'std::uninitialized_copy<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_vector.h:322:9: note||in instantiation of function template specialization 'std::__uninitialized_copy_a<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > >' requested here|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp:33:20: note||in instantiation of member function 'std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > >::vector' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unique_ptr.h|356|note: 'unique_ptr' has been explicitly marked deleted here|
||=== Build failed: 2 error(s), 19 warning(s) (0 minute(s), 9 second(s)) ===|

Le problème avec les pointeurs intelligent c'est que ça me met la ligne d'erreur dans le fichier std::unique_ptr et non pas dans mon code...

[mintho carmo]

Faut pas exagérer. La STL n'est pas parfaite, les pré-conditions ne sont pas forcement vérifiées et les messages d'erreur ne sont pas toujours explicite (cela devrait s'améliorer avec les concepts), mais il est difficile de critiquer les devs de la STL alors que la plupart des développeurs font pareil.

En plus, la pile d'erreur permet souvent de retrouver la fonction dans la code appelant qui pose problème. Dans ton cas, c'est un appel de make_unique sur Entity, dans cellMap.cpp à la ligne 19 :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp:19:41: note||in instantiation of function template specialization 'std::make_unique<odfaeg::graphic::Entity, odfaeg::graphic::Entity *>' requested here|

Sinon, les warning ne sont pas décoratifs, il faut les régler. Tu en particulier des problèmes de masquage de fonction, utilises override.

HS : tes fonctions qui posent problème dans Entity (isLight, isShadow, etc) me fait penser à un problème de conception. Ta classe Entity semble devoir connaitre le type réel des classes enfants manipulées, ce qui ne va pas

[Bousk]

Le problème c'est surtout que tu remets toujours promptement le reste du monde en question avant toi-même. Hors ton historique prouve que l'inverse est vrai dans 99.99% des cas..
En l'occurence l'erreur est plutôt bien visible /home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp:19:41: note||in instantiation of function template specialization 'std::make_unique<odfaeg::graphic::Entity, odfaeg::graphic::Entity *>' requested here|, encore faut-il lire la sortie.

[Invité]

C'est surtout que je vais devoir essayer de limiter le nombre de messages d'erreurs sur la sortie, parce que je ne m'y retrouve plus! Bref...

Je fait en effet un std;;make_unique ici :

Code cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
void CellMap::addEntity (Entity *entity) {
            entityInside.push_back(std::make_unique<Entity>(std::move(entity)));
        }

Mais je ne parviens pas à changer la propriété du pointeur.

Si je retire le std::make_unique<Entity> il me dit ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
||=== Build: Debug in ODFAEG (compiler: GNU GCC Compiler) ===|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/fastDelegate.h|170|warning: 'RefVal' defined as a struct template here but previously declared as a class template [-Wmismatched-tags]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/fastDelegate.h|70|note: did you mean struct here?|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/resourceManager.h|528|warning: unused variable 'id' [-Wunused-variable]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|60|warning: 'odfaeg::physic::BoundingVolume::intersects' hides overloaded virtual functions [-Woverloaded-virtual]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|36|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingBox &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|37|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingSphere &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|38|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingEllipsoid &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|39|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::OrientedBoundingBox &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/boundingVolume.h|40|note: hidden overloaded virtual function 'odfaeg::physic::BaseInterface::intersects' declared here: type mismatch at 1st parameter ('odfaeg::physic::BoundingPolyhedron &' vs 'odfaeg::physic::BaseInterface &')|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|187|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|292|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|warning: '&&' within '||' [-Wlogical-op-parentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Physics/../Math/computer.h|416|note: place parentheses around the '&&' expression to silence this warning|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/../../../include/odfaeg/Graphics/../Math/../../../include/odfaeg/Core/stateParameter.h|32|warning: field 'name' will be initialized after field 'value' [-Wreorder]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp|19|error: no matching member function for call to 'push_back'|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_vector.h|913|note: candidate function not viable: no known conversion from 'typename std::remove_reference<Entity *&>::type' (aka 'odfaeg::graphic::Entity *') to 'const value_type' (aka 'const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >') for 1st argument|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_vector.h|931|note: candidate function not viable: no known conversion from 'typename std::remove_reference<Entity *&>::type' (aka 'odfaeg::graphic::Entity *') to 'value_type' (aka 'std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >') for 1st argument|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp|62|warning: comparison of unsigned expression >= 0 is always true [-Wtautological-compare]|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_construct.h|75|error: call to deleted constructor of 'std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >'|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:75:8: note||in instantiation of function template specialization 'std::_Construct<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > &>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:125:2: note||in instantiation of function template specialization 'std::__uninitialized_copy<false>::__uninit_copy<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_uninitialized.h:278:19: note||in instantiation of function template specialization 'std::uninitialized_copy<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *>' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/stl_vector.h:322:9: note||in instantiation of function template specialization 'std::__uninitialized_copy_a<__gnu_cxx::__normal_iterator<const std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > > >, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > *, std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > >' requested here|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Graphics/cellMap.cpp:33:20: note||in instantiation of member function 'std::vector<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> >, std::allocator<std::unique_ptr<odfaeg::graphic::Entity, std::default_delete<odfaeg::graphic::Entity> > > >::vector' requested here|
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unique_ptr.h|356|note: 'unique_ptr' has been explicitly marked deleted here|
||=== Build failed: 2 error(s), 16 warning(s) (0 minute(s), 1 second(s)) ===|

[mintho carmo]

A priori, tu n'as pas compris le rôle de make_unique et de la move semantic.
Mais peut importe en fait ici, il y a un problème de conception : pourquoi un objet managé par un unique_ptr est d'abord créé dans un premier temps comme pointeur nu ?
Sois tu crées une fonction createEntity qui va créer ton entité dans ta map et retourner un unique_ptr<Entity>&, sois (si tu ne veux pas donner la responsabilité de la création à ta map) tu prends un unique_ptr<Entity> comme paramètre de addEntity (ou une référence sur un unique_ptr<Entity> si ta map n'est pas responsable de la durée de vie de tes entités et dans ce cas, tu dois utiliser un vector<reference_wrapper<Entity>> dans ta map)

C'est surtout que je vais devoir essayer de limiter le nombre de messages d'erreurs sur la sortie, parce que je ne m'y retrouve plus!

J'espère que ton idée est de corriger le code et pas simplement supprimer les options de vérification du compilateur ?!

[Invité]

A priori, tu n'as pas compris le rôle de make_unique et de la move semantic.
Mais peut importe en fait ici, il y a un problème de conception : pourquoi un objet managé par un unique_ptr est d'abord créé dans un premier temps comme pointeur nu ?

Car je ne veux pas avoir de fonction qui prenne des pointeurs intelligent dans mon framework, je veux que la gestion de la mémoire soit totalement transparente, exactement comme avec le framework QT.

En fait j'ai des classes RAII avec des std::vector ou std::map qui se chargent elle même de libérer ou bien d'allouer de la mémoire.

J'espère que ton idée est de corriger le code et pas simplement supprimer les options de vérification du compilateur ?!

En ce moment je cherche surtout à intégrer les pointeurs intelligents plutôt que de gérer la libération de la mémoire dynamiquement.

Sinon mon code fonctionne et je m'y retrouve bien, c'est le principal, je suis même parvenu à créé un pong avec mon moteur et un RPG (mais je dois encore implémenter le gameplay)

[mintho carmo]

Car je ne veux pas avoir de fonction qui prenne des pointeurs intelligent dans mon framework, je veux que la gestion de la mémoire soit totalement transparente, exactement comme avec le framework QT.

Qt ne gère absolument pas la mémoire de façon transparente. Elle (oui, j'ai décidé que Qt est une fille) utilise plusieurs modèles de mémoire (ie des objets managés avec le système de parent-enfants et d'autre non managés). Sauf que dans les 2 cas, cela utilise des pointeurs nus, ce qui ne permet pas de savoir avec l'API que les objets seront managés ou non. Et donc Qt ne peut pas tester si un objet est managé ou non, ie que Qt est utilisé correctement ou non. Il faut comprendre le fonctionnement interne de Qt pour savoir ce qui est managé ou non (ce qui produit régulièrement de erreurs).

Un "bon" framework permettra de savoir explicitement ce qui est managé ou pas et vérifia qu'il est utilisé correctement. Si tu prends un pointeur nu et que tu le manage, cela veut dire que quelqu'un peut faire l'erreur de fournir un objet et le manager lui même en pensant qu'il n'est pas managé par ton framework = problème.
Surtout que finalement, ton approche est un "mensonge", dans le sens où tu crées une API qui est compatible avec des objets non managés, alors qu'en fait tu ne peux pas utiliser des objets managés dans ton framework. C'est pas du tout un modèle transparent.

(et oui, Qt n'est pas un "bon" framework dans ce sens. C'est un "très bon" framework pour pleins de choses, mais il est ancien et donc basé sur l'ancien modèle mémoire du C++. D'ailleurs, Qt implémente des pointeurs intelligents bien avant le C++11 et pourtant ne les utilisent pas dans son modèle mémoire. Tout simplement parce que cela pose problème).

Donc, si tu veux faire les choses proprement, sois tu utilises explicitement des types managés si ton framework prend en charge le management des objets, soit tu utilises des types non managés si ton framework prend pas en charge les objets.
Si tu veux que ce soit transparent (ie pas de type managé dans l'API de ton framework), alors tu ne dois pas manager les objets. Libre à chaque d'utiliser les outils de gestion des objets ou non derrière.

En fait j'ai des classes RAII avec des std::vector ou std::map qui se chargent elle même de libérer ou bien d'allouer de la mémoire.

Un simple exemple bête : si tu as un objet dans un vector et que tu l'utilises dans ta fonction addEntity, tu provoques un crash :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
vector<Entity> entities;
...
map.addEntity(&entities[i]); // problème

Bref, oublie le côté "transparent", ton framework ne l'est pas.

Sinon mon code fonctionne

Tout dépend de tes critères. Pour moi, un code "fonctionne" quand il faut ce que l'on attend de lui. ie il passe tous les tests (unitaire et fonctionnaire et autres) sans problèmes. Une erreur/warning est un problème, donc suffit à rejeter un code (j'utilise -Werror, qui provoque une erreur lorsque j'ai un warning. Donc un warning = erreur de build = arrêt des tests = échec)

[Bousk]

Moi j'ai jamais utilisé ce genre de trucs, mais Google est plutôt explicite sur la doc de std::make_unique et à aucun moment il n'apparait un semblant de prototype que tu sembles espérer utiliser...
Par contre std::unique_ptr possède bien ça dans sa liste de constructeurs.

[mintho carmo]

Moi j'ai jamais utilisé ce genre de trucs, mais Google est plutôt explicite sur la doc de std::make_unique et à aucun moment il n'apparait un semblant de prototype que tu sembles espérer utiliser...
Par contre std::unique_ptr possède bien ça dans sa liste de constructeurs.

Pas sûr de comprendre à quel message tu réponds

[Invité]

J'ai essayé avec le constructeur de std::unique_ptr et même avec la méthode reset, mais, ça ne fonctionne pas.

Peut être devrais-je utiliser des delete et des destructeurs comme c'était le cas à l'ancienne.

[Iradrille]

Entièrement d'accord avec mintho carmo.

Avec quelque chose comme ça void CellMap::addEntity (Entity *entity) { }, on ne donne aucune indication sur la possession du pointeur.
Ça laisse penser que la fonction utilise le pointeur en assumant qu'il reste valide mais ne s'occupe pas de sa destruction.

Dans ce cas

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
struct Entity {};
Entity e;
CellMap cm;
cm.addEntity(&e);

serait tout à fait valide (et plantera lamentablement quand esera delete).

Maintenant avec l'utilisation de unique_ptr : void CellMap::addEntity (std::unique_ptr<Entity>&& e) { }.

Ici, on donne une indication : on "abandonne" l'objet, il sera géré par la fonction.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
struct Entity {
   Entity(int, int, int) {}
};
CellMap cm;
auto e = std::make_unique<Entity>(1, 2, 3); // make_unique forward les paramètres au constructeur d'Entity et se charge de la création de l'objet.
cm.addEntity(std::move(e)); // il est clair qu'ici on donne la possession du pointeur.

[Invité]

Entièrement d'accord avec mintho carmo.

Avec quelque chose comme ça void CellMap::addEntity (Entity *entity) { }, on ne donne aucune indication sur la possession du pointeur.
Ça laisse penser que la fonction utilise le pointeur en assumant qu'il reste valide mais ne s'occupe pas de sa destruction.
struct Entity {};
Entity e;
CellMap cm;
cm.addEntity(&e);

serait tout à fait valide (et plantera lamentablement quand esera delete).

Je peux tester si e est valide (si il est différent de nullptr) et comme c'est un pointeur, je dois faire une allocation dynamique.

Donc, je ne peux pas envoyer l'adresse d'un objet non alloué dynamiquement dans le framework, toute entité est allouée dynamiquement en dehors du framework et est détruite lors de l'arrêt de l'application, et si il n'y a pas d'héritage alors je fais une copie!

C'est le framework lui même qui sert de capsule RAII, je préfère cela car ça rend la syntaxe beaucoup moins verbeuse.

[Iradrille]

Je peux tester si e est valide (si il est différent de nullptr) et comme c'est un pointeur, je dois faire une allocation dynamique.

Donc, je ne peux pas envoyer l'adresse d'un objet non alloué dynamiquement dans le framework, toute entité est allouée dynamiquement en dehors du framework et est détruite lors de l'arrêt de l'application, et si il n'y a pas d'héritage alors je fais une copie!

Je vois pas ce qui t'empêches d'allouer l'objet sur la pile et de fournir son adresse à ton framework ? Un pointeur est un pointeur, c'est une adresse rien d'autre. Il n'y a pas d'indication quant à où est alloué l'objet (statiquement sur la pile ou dynamiquement sur le tas).

Regarde cet exemple minimaliste

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
#include <vector>
#include <memory>
 
struct Entity {
	virtual void foo() = 0; // Entity est non instanciable
	virtual ~Entity() { }
};
 
typedef std::unique_ptr<Entity> EntityPtr;
 
struct EntityA : Entity {
	virtual void foo() override { }
};
 
struct CellMap {
 
	std::vector<EntityPtr> entities;
 
	void addEntity(Entity *e) {
		// on peut empêcher l'insertion de nullptr,
		//ou tester lors de l'utilisation
		if(e) {
			entities.emplace_back(e);
		}
	}
};
 
int main() {
	EntityA e;
	{
		CellMap cm;
		cm.addEntity(&e);
	} // destruction de cm, et donc appel de "delete e" via l'unique_ptr -> stack trashed
 
	return 0;
}

On utilise bien des pointeurs et le polymorphisme, rien n'indique que la fonction prend possession de l'objet pointé : on est donc "en droit" de fournir un pointeur sur un objet alloué sur la pile; tant que l'objet vis plus longtemps que la CellMap qui l'utilise ça ne devrait pas poser de problèmes.

Mais CellMap prend possession du pointeur et le delete.

Le résultat :


On peut se retrouver avec le même problème avec des std::unique_ptr, mais c'est clairement la faute de l'utilisateur :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
#include <vector>
#include <memory>
 
struct Entity {
	virtual void foo() = 0; // Entity est non instanciable
	virtual ~Entity() { }
};
 
typedef std::unique_ptr<Entity> EntityPtr;
 
struct EntityA : Entity {
	virtual void foo() override { }
};
 
struct CellMap {
 
	std::vector<EntityPtr> entities;
 
	void addEntity(EntityPtr&& e) {
		// on peut empecher l'insertion de nullptr,
		//ou tester lors de l'utilisation
		if(e) {
			entities.emplace_back(std::move(e));
		}
	}
};
 
int main() {
	EntityA e;
	{
		CellMap cm;
		// pas de custom deleter fourni, erreur de l'user.
		EntityPtr pe(&e);
 		cm.addEntity(std::move(pe));
	} // destruction de cm, et donc appel de "delete e" via l'unique_ptr -> stack trashed
 
	return 0;
}

Mais dans ce cas

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
EntityA e;
EntityPtr pe(&e);

Est plus que suspect.

Une utilisation normale serait

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
int main() {
	CellMap cm;
 
	cm.addEntity(std::make_unique<EntityA>(/* params du ctor de EntityA */));
 
	// ou, si tu est bloqué sur un compilo sans std::make_unique (genre VS2012)
	// EntityPtr pe(new EntityA);
 	// cm.addEntity(std::move(pe));
 
	return 0;
}

C'est le framework lui même qui sert de capsule RAII, je préfère cela car ça rend la syntaxe beaucoup moins verbeuse.

un new EntityX(/* params */); vs un std::make_unique<EntityX>(/* params */);, le code n'est pas beaucoup plus court. =)

[mintho carmo]

On va rien dire ou te mettre de -1, tu vas encore croire a un complot contre toi. C'est ton framework, tu es libre de tes choix de design, même si plusieurs personnes pensent que c'est une mauvaise idée (et qu'on t'a montré des codes qui pouvait être problématique)

Mais on ne peut pas faire de miracle. Si tu as une classe non instantiable et non copiable, tu ne peux pas l'instancier ou la copier. C'est comme cela. Soit tu la rend instanciable et copiable, soit tu ne l'instancie pas et la copie pas.

Si tu veux persister dans ton idee :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
entityInside.push_back(std::move(std::unique_ptr<Entity>(entity));

(remarque : créer un unique_ptr a partir d'un pointeur nu est très très basique. Si tu sais pas faire cela, j'ai du mal a croire que tu maîtrise suffisamment les pointeurs pour dire que ton API est correcte, alors que d'autres plus expérimenté te disent non)

[Invité]

Ok vous deux vous ne me raconter que des salades.

Je me suis arrêté à la première phrase, j'ai pas été plus loin :

Je vois pas ce qui t'empêches d'allouer l'objet sur la pile et de fournir son adresse à ton framework ? Un pointeur est un pointeur, c'est une adresse rien d'autre. Il n'y a pas d'indication quant à où est alloué l'objet (statiquement sur la pile ou dynamiquement sur le tas).

[Iradrille]

J'en profite, y a-t-il une différence entre ces 2 codes ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

entityInside.push_back(std::move(std::unique_ptr<Entity>(entity));

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

entityInside.emplace_back(entity);

L'un est plus court (héhé, ça m'a pris du temps à trouver ça..), mais au final le binaire généré devrait être strictement identique non ?

@Lolilolight, je me demande si c'est de la mauvaise foi, ou si quelque chose m’échappe.

Je peux tester si e est valide (si il est différent de nullptr) et comme c'est un pointeur, je dois faire une allocation dynamique.

Peux-tu expliquer ça ? En quoi un pointeur est synonyme d'allocation dynamique ?