Discussion :

[Bktero]

Bonjour,

J'ai un code qui ressemble à ça :

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
#include <array>
#include <iostream>
 
using byte = std::uint8_t;
 
template<std::size_t __size>
using ControlBlock = std::array<byte, __size>;
 
class Base {
public:
	template<std::size_t __size>
	Base(ControlBlock<__size>& controlBlock) :
			buffer_m(controlBlock.data()),
			size_m(__size) {
	}
 
	void start() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		buffer_m[0] = 42;
	}
 
private:
	byte *buffer_m;
	std::size_t size_m;
};
 
class Derived : public Base {
public:
	Derived() :
			Base(cb_m) {
	}
 
private:
	ControlBlock<512> cb_m;
};
 
int main() {
	ControlBlock<128> buffer;
	Base base = Base(buffer);
	base.start();
 
	Derived derived;
	derived.start();
}

J'ai une classe fille qui initialise sa partie mère en lui passant un de ces champs. C'est un peu border line car la partie dérivée (et donc le champ en question) n'est pas construite à ce moment, mais ici il s'agit d'une classe (ControlBlock) qui ne contient rien qui ait besoin d'une initialisation. Mon vrai ControlBlock contient une structure C (donc sans constructeur), un tableau (un vrai, pas un std::array) et une taille (en static constexpr). Il se mime donc assez bien avec un std::array, d'où mon exemple minimaliste. Sauf que mon exemple ne reproduit pas mon problème : dans ma vraie application, ça crashe dés que je tente d'utiliser mon ControlBlock dans la méthode start().

Est-ce totalement illégal de faire ce que je fais ou est-ce que j'ai une chance que ça puisse fonctionner ?

Merci d'avance !

[Bousk]

Je pense que ton crash vient surtout du fait que ton paramètre est une copie, donc tu prends le pointeur d'une copie et d'un temporaire.
Tu devrais plutôt passer le pointeur en paramètre.

[Garvelienn]

Bonjour,

J'aurais tendance à dire que c'est en effet très borderline. Est-ce que vous n'avez aucune autre architecture possible dans votre cas d'utilisation ?

Pour répondre à la question, quelqu'un de plus calé saura mieux répondre, mais j'essaye quand même. Lorsque le constructeur de la classe fille est appelé, le constructeur de la classe parent est aussitôt appelé. Et c'est seulement après que les variables de la classe fille sont initialisées. Du coup, le constructeur de la classe parente se retrouve avec une variable non-initialisée ce qui induit un comportement inconnue et différent en fonction de l'état de la RAM.

Il y a un ordre bien précis à respecter dans la liste d'initialisation des constructeurs en C++. Plus d'info ICI et ICI

[Bktero]

@Bousk : au temps pour moi, c'est une erreur du code minimaliste que je poste. Le constructeur prend une référence en paramètre. J'ai modifié mon code en conséquence....qui ne crashe toujours pas.

@Garvelienn : c'est effectivement parce que je sais que ça s'initialise dans cet ordre que ça me fait peur Quand à une autre solution, j'aurais bien aimé mais le code réel à beaucoup de contraintes (il s'interface avec une API C de gestion de threads).

J'ai trouvé une solution de contournement : j'ai rajouté en protected un constructeur par défaut et une méthode initialize() qui fait tout le travail du constructeur public (qui maintenant se contente d'appeler initialize()). Ainsi les classes peuvent initialiser leur base part dans la liste d'initialisation, puis appeler initialize() dans le corps de leur propre constructeur. A ce moment là, le champ tcb_m a été construit et tout se passe bien. Ca ressemble à ça :

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
45
46
47
48
49
50
51
52
53
#include <iostream>
#include <array>
 
using byte = std::uint8_t;
 
template<std::size_t __size>
using ControlBlock = std::array<byte, __size>;
 
class Base {
public:
	template<std::size_t __size>
	Base(ControlBlock<__size> &controlBlock) {
		initialize(controlBlock);
	}
 
	void start() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		buffer_m[0] = 42;
	}
 
protected:
	Base() = default;
 
	template<std::size_t __size>
	void initialize(ControlBlock<__size> &controlBlock) {
		buffer_m = controlBlock.data();
		size_m = __size;
	}
 
private:
	byte *buffer_m;
	std::size_t size_m;
};
 
class Derived : public Base {
public:
	Derived() :
			Base() {
		initialize(tcb_m);
	}
 
private:
	ControlBlock<512> tcb_m;
};
 
int main() {
	ControlBlock<128> buffer;
	Base base = Base(buffer);
	base.start();
 
	Derived derived;
	derived.start();
}

[Garvelienn]

C'est ce que j'aurais fait aussi dans ton cas. La fonction initialize() te permet mieux contrôler l'ordre des initialisations.

Cependant, par pure curiosité, il serait intéressant de savoir quel impact cela a sur les performances. Plus d'appels/initialisations avec la fonction initialize() qu'avec une initialisation par membres avant l'appel du corps du constructeur ?

[Luc Hermitte]

Code c++ :

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
struct Holder {
    Type m_machine;
};
 
class Base : noncopyable {
public:
    Base(Type & machin);
    virtual ~Base() = default;
};
 
class Child : Holder, public Base // ATTENTION: ne jamais changer l'ordre de ces parents
{
public:
    Child() : Base(Holder::m_machine) // ATTENTION: repose sur l'ordre des parents: Holder, puis Base
   {}
};

C'est tordu, mais c'est un moyen de s'en sortir: la partir Holder est construite avant. C'est le genre de choses à commenter abondamment si on veut avoir une chance que cela passe les contrôles qualité.
On peut aussi imaginer que Holder nécessite un constructeur qui ne peut évidemment pas se reposer sur des choses définies dans le constructeur de Child.

[Pyramidev]

Bonjour.

Pour contrôler l'ordre d'initialisation des données de la classe, le plus propre (et aussi le plus simple) serait de faire de la composition :

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
45
46
47
48
49
50
51
52
53
#include <array>
#include <iostream>
 
using byte = std::uint8_t;
 
template<std::size_t __size>
using ControlBlock = std::array<byte, __size>;
 
class StartableBufferProxy final {
public:
	template<std::size_t __size>
	explicit StartableBufferProxy(ControlBlock<__size>& controlBlock) :
		buffer_m{controlBlock.data()},
		size_m{__size}
	{}
	void start() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		buffer_m[0] = 42;
	}
private:
	byte* buffer_m;
	std::size_t size_m;
};
 
class StartableBuffer final {
public:
	StartableBuffer() :
		cb_m{}, bufferProxy_m{cb_m}
	{}
	void start() {
		bufferProxy_m.start();
	}
	// Viol de l'encapsulation :
	// Fonction à n'ajouter que si on veut qu'une référence de type StartableBufferProxy&
	// puisse référer le StartableBufferProxy contenu dans StartableBuffer, comme dans
	// le cas où StartableBuffer dériverait de StartableBufferProxy :
	StartableBufferProxy& getInnerBufferProxy() {
		return bufferProxy_m;
	}
private:
	ControlBlock<512> cb_m;
	StartableBufferProxy bufferProxy_m; // doit être déclaré après cb_m
};
 
void testWithoutInheritance() {
	ControlBlock<128> buffer;
	StartableBufferProxy obj1{buffer};
	StartableBuffer      obj2{};
	StartableBufferProxy& ref1 = obj1;
	StartableBufferProxy& ref2 = obj2.getInnerBufferProxy();
	ref1.start();
	ref2.start();
}

Si on veut faire de l'héritage, on peut toujours ajouter des classes par dessus :

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
45
46
47
48
class Startable {
public:
	virtual void start() = 0;
protected:
	Startable()                            = default;
	~Startable()                           = default;
	Startable(Startable const&)            = default;
	Startable(Startable&&)                 = default;
	Startable& operator=(Startable const&) = default;
	Startable& operator=(Startable&&)      = default;
};
 
class DerivedStartableBufferProxy final : public Startable {
public:
	template<std::size_t __size>
	explicit DerivedStartableBufferProxy(ControlBlock<__size>& controlBlock) :
		inner_m{controlBlock}
	{}
	void start() override {
		inner_m.start();
	}
private:
	StartableBufferProxy inner_m;
};
 
class DerivedStartableBuffer final : public Startable {
public:
	DerivedStartableBuffer() :
		inner_m()
	{}
	void start() override {
		inner_m.start();
	}
private:
	StartableBuffer inner_m;
};
 
// On évite le viol de l'encapsulation fait par StartableBuffer::getInnerBufferProxy().
// Mais appeler Startable::start() ajoute le coût de la virtualité.
void testWithInheritance() {
	ControlBlock<128> buffer;
	DerivedStartableBufferProxy obj1{buffer};
	DerivedStartableBuffer      obj2{};
	Startable& ref1 = obj1;
	Startable& ref2 = obj2;
	ref1.start();
	ref2.start();
}

Bktero, pour quelle raison utilises-tu de l'héritage ? Y a-t-il du polymorphisme dans le code appelant ? Si oui, de quelle manière ?

Edit 2018-07-14-20h04 : renommage des fonctions main en testWithoutInheritance et testWithInheritance.

[Bktero]

Merci pour vos idées. Je me rend compte en lisant vos propositions que je n'avais sans doute pas assez explicité mon cas réel, du coup vos solutions ne collent pas tout à fait. Mais ça pourrait toujours être utile dans d'autres codes tordus à venir Je marque comme résolu, la solution avec initialize() devrait convenir pour la suite !