samedi, août 28, 2010

-Weffc++

Le compilateur C++ peut s'avérer pénible par moment, et n'aide pas vraiment le pauvre programmeur du midi que je suis à éviter les bourdes. D'où l'intérêt (théorique) d'un peu plus d'avertissements pour vérifier qu'un certain nombre de "règles de bonne pratique" sont appliquées. Voyons un peu ce que ça donne dans le "code de Bilou"...

base class 'class UsingScript' has a non-virtual destructor
Le but de ces classes "UsingXxx" est de partager entre plusieurs classes un pointeur statique (en l'occurence, l'instance de GameScript) tout en ayant la possibilité d'être averti lorsque ce pointeur n'est plus valide ou de le mettre à jour. Aucun élément par instance, donc. Rien à détruire. Je n'ai pas écrit de destructeur, mais celà ne semble pas suffisant au compilateur: ce code serait plus "efficient" si mon non-destructeur était virtuel :P

'class iGobController' has pointer data members
Oui, en effet, et c'est bien naturel. "efficient C++" voudrait dès lors que je surcharge iGobController(const iGobController&), le constructeur de copie, ni operator=(const iGobController&), l'opérateur d'affectation. Moi, je voudrais surtout pouvoir dire en au plus une ligne "gueule un coup si et seulement si j'essaie de copier un iGobController qui n'est pas prévu pour ça". Mieux, j'aurais voulu dire "SpritePage implements Copiable", et ne rien dire pour les 90% de mes classes, mais ça, c'est du java et je ne code pas en Java sur DS ...

Et qu'on ne s'y trompe pas: ce sont bien les deux que C++ réclame: constructeur de copie et opérateur d'affectation. Si le second peut facilement être remplacé par du "faux code privé" uniquement destiné à produire une erreur si j'essayais de copier quelque-chose de non-copiable, coder le premier nécessite de donner une valeur à tous les membres, ce qui tue complètement l'intérêt de la chose... mais il y aurait tout de même une astuce: donner uniquement le prototype des copieurs en tant que variables privées, sans leur donner d'implémentation.

#ifndef NOCOPY
#define NOCOPY(__x) __x& operator=(const __x &that);\\
__x(const __x &that);
#endif

'ListOfFiles::tnext' should be initialized in the member initialization list
C'est sans doute la fonctionnalité qui m'intéresse le plus dans -Weffc++ : détecter les membres non-initialisés dans les constructeurs. A développer une demie-heure par jour ou entre la mise au lit de *deline et la mienne, c'est le genre de bourde qui est courante et qui passe inaperçue sous émulateur (dont la mémoire est joliment pleine de 0 quand le programme démarre). Notez qu'ici, j'avais ListOfFiles() { tnext=tank; } alors que -Weffc++ aurait voulu me voir écrire ListOfFiles() tank(), tnext(tank), lof() {}. Pas convaincu.

warning: 'ListOfFiles::lof' should be initialized in the member initialization list
Ah oui, parce que, bien sûr, l'initialisation par défaut de mon vecteur n'aura lieu que si je le précise explicitement. Bjarne, tu me rappelles la signification de "par défaut", stp ?
Allez, reconnaissons-lui quand-même que si tank est un tableau plutôt qu'un vecteur, : tank() a au moins l'avantage d'être plus concis (et pose moins de risque d'erreurs) que memset(tank,0,sizeof(tank)); ... et curieusement, celui-là, personne ne s'est plaint que je ne l'avais pas initialisé, tiens!

base class 'class std::vector<...>' has a non-virtual destructor
Argh. C'était peut-être une très mauvaise idée de construire quelque-chose qui soit une sous-classe de vector dès le départ mais ... On fait quoi, dans ces cas-là ? Help! Stack Overflow !

happyhttp::Response::m_Length' will be initialized after 'std::string happyhttp::Response::m_VersionString'
et ?

En conclusion: pas bien concluant, tout ça. Seuls certains des soucis couramment rencontrés sont pris en charge et je me retrouve avec plus de nouveaux problèmes que de nouvelles solutions. Bref, il y a peut-être un cas de figure qui justifie l'existence de C++, mais jusqu'ici, je ne l'ai pas encore vu. 'vais continuer de chercher, hein. ++A.

3 commentaires:

Cyril a dit…

Destructeur non-virtuel. Il ne s'agit pas d'une question d'efficacité mais de sécurité. Si une sous-classe de UsingScript a besoin d'un destructeur mais est adressée via une variable ou un conteneur dont le type déclaré est UsingScript, c'est le destructeur vide de UsingScript qui sera appelé à sa destruction et les ressources ne seront pas libérées.

Pointer data members. Cet aspect est pénible en C++, mais on ne peut pas en vouloir à -Weffc++ d'attirer notre attention sur cette source d'erreur fréquente. Il suffit de déclarer le constructeur de copie et l'opérateur d'affectation comme étant privé. Je n'ai pas compris la macro présentée.

Initialization list. Ici, il ne s'agit pas d'une question de sécurité (je n'ai pas oublié de variable) mais d'une question d'efficacité. Sans liste d'initialization, le compilateur va utiliser les constructeurs par défaut de tous les membres. Certains de ces objets seront ensuite remplacés dans le corps du construteur par un nouvel objet. Certains auront donc été construits inutilement. Via la liste d'initialisation, on affecte directement l'objet final au membre. Cependant, on peut regretter que cet avertissement soit aussi valable pour des types primitifs sans constructeurs et qu'il n'analyse pas le corps de la fonction pour détecter les cas où l'objet construit par défaut est utilisé tel quel.

sylvainulg a dit…

@Cyril: merci pour tes compléments d'informations. Quelques petits tests (guidés aussi par les réponses obtenues sur Stack Overflow) n'ont pas l'air terriblement convaincants malgré tout.

class UsingScript {
 protected:
   static char* name;
 public:
   static void setscript(char *scr) { name=scr; }
};

class Object : private UsingScript {
 public:
   virtual void doit() {
     printf("with script %s",name);
   }
   virtual ~Object() {}
};

avec ces 2 classes, impossible de convertir un Object en UsingScript, puisqu'il s'agit d'une "base inaccessible". Ca n'empêche pas gcc de trouver "efficient" de se plaindre du non-destructeur non-virtuel de UsingScript. Bref, je persiste et signe: ce warning n'analyse pas suffisamment la situation, et contrairement aux warnings-de-bienscéance du langage C (genre "suggest parentheses around assignment used as truth value"), le warning ne me donne aucune piste pour améliorer. J'aurais donc tendance à dire que -Weffc++ est à réserver aux lecteurs assidûs du bouquin "Efficient C++".

sylvainulg a dit…

class Widget {
   NOCOPY(Widget);
 public:
   // ... some stuff
};

pour faire de Widget une classe qu'il n'est pas possible de copier parce que les opérateurs & constructeurs prévus à cet effet sont privés.