Journal compiler en c++ pour avoir plus de warnings

Posté par .
Tags :
12
15
nov.
2012

Ca parait un peu foireux, mais pourtant android le font https://android-review.googlesource.com/#/c/40939/

commit de727caee24df86c3052508aa213f7165168913a
Author: Elliott Hughes
Date: Mon Aug 13 15:45:36 2012 -0700

Clean up warnings in stubs.cpp.

Switch to C++ to get extra warnings, and format the code Google style.

Change-Id: Ifc0131bf297a6ee8a8a6b8d049a02518b0b1a4b7

Qu'en pensez vous ?

PS : oui on est pas encore vendredi

  • # Les warnings

    Posté par . Évalué à 6.

    Ça peut ennuyer les débutants ou les étudiants. Après plus tu as perdu un temps monstrueux à essayer de déboger un code sur une bêtise, plus tu as tendance à comprendre leur intérêt, à les corriger systématiquement sauf dans des cas très particulier si tu peut pas vraiment faire autrement et à chercher le maximum de vérification automatique qui coûtent pas cher.

    Enfin je parle pour moi mais j'imagine que je ne suis pas tout seul.

    • [^] # Re: Les warnings

      Posté par . Évalué à 3.

      Pareil,
      Si il y a un warning c'est qu'il y a une raison,
      Et si cette raison peut une fois sur 1000 entrainer un problème, autant s'en débarasser le plus tôt possible.

      D'ailleurs pour les mêmes raisons j'use et j'abuse du #warning

      Quel plaisir de faire raler un collègue avec

      #warning What the hell is that f… formula, documentation needed !!!

    • [^] # Re: Les warnings

      Posté par (page perso) . Évalué à 9.

      Sur certains projets persos, on s'est mis d'accord pour interdire les commits où le code génère des warnings à la compilation.

      Voici mon hook git (pre-commit) facilement adaptable (et au passage, si on oublie de faire un add sur un fichier nécessaire pour la compilation, ça empêche le commit) :

      #!/bin/sh
      #
      # to ignore check : --no-verify
      
      against=`git rev-parse --verify HEAD`
      
      # Creation d'un dossier temporaire.
      TMPDIR=`mktemp -d`
      # On copie le diff dans un fichier patch.
      git diff $against > $TMPDIR/current_change.patch
      # On copie le HEAD.
      git archive $against | tar -x -C $TMPDIR
      # on va dans le dossier temporaire pour appliquer le patch.
      cd $TMPDIR
      git apply current_change.patch
      # make depend
      make depend
      # on compile avec -Werror.
      make -j16 WERROR=1
      
      rc=$?
      
      rm -rf $TMPDIR
      
      if [ $rc -ne 0 ]; then
      echo "Erreur !"
      exit $rc
      fi
      
      
      • [^] # Re: Les warnings

        Posté par (page perso) . Évalué à 1.

        J'imagine le commit qui dure 3 heures quand tu modifies un pauvre espace dans un projet qui compte 1 ou 2 millions de LOC.

        Rassure moi, tes projets compilents en moins de 5 secondes?

        • [^] # Re: Les warnings

          Posté par . Évalué à 2.

          Sauf dans le cas d'un projet perso c'est clairement pas au VCS de faire ça.

          Si tu fonctionnes par patch c'est à la phase en amont de faire ça (tu retrouves ce fonctionnement chez les projets Apache où un rapport sur les patchs attachés au BTS est généré. Comme ça tu peux décider de le merger ou de le raffiné en fonction de son score). Et dans tout les cas tu valides les branches en intégration continue.

          Dans tout les cas le but étant de minimiser les problèmes avant et de les corriger rapidement après. Avoir une politique bête, méchante et binaire sur un hook de commit qui empêche d'avancer est souvent contre productive.

          • [^] # Re: Les warnings

            Posté par (page perso) . Évalué à 2.

            Je réponds aux 2 commentaires. J'ai commencé par "sur certains projets persos", donc ça dépend fortement du projet.

            Le temps de compilation du projet où j'ai repris ce morceau de code est de l'ordre de 30 secondes donc c'est tout à fait acceptable. Avant de mettre en place ce système, on avait des problèmes du type "putain, il a oublié un fichier dans son commit, il a tout cassé" ou encore "bon allé, faut vraiment qu'on dégage tous les warnings, qui s'en charge ?". Au final, le temps perdu (30s par commit) est largement compensé par le temps économisé (attente que le contributeur revienne pour qu'il ajoute le fichier manquant, ou revert de ses derniers commits) ou sur la qualité du code (tu essayes de faire un joli commit qui compile sans warning, adieu le : "ouais bon, je corrigerai les warnings plus tard").

            Sur des projets non persos, j'ai déjà vu dans de grosses boites un check avec un linter avant chaque commit. Et le faire avant que le code ne soit versionné est bien sûr contraignant car il empêche d'avancer, mais il permet aussi de ne pas avoir à y revenir.

            La méthode que tu décris est bien sûr meilleure mais ce n'est pas applicable à chaque projet.

            • [^] # Re: Les warnings

              Posté par . Évalué à 2.

              La méthode que tu décris est bien sûr meilleure mais ce n'est pas applicable à chaque projet.

              Ça fait longtemps que je crois plus au relation d'ordre dans le développement. Le mot clé est "pragmatisme" ;)

              Je comprends pourquoi tu fais ca. D'une part par ce que c'est facile et d'autre part c'est une contrainte ferme. Par contre je souligne que généralement on fait ça en amont ou en aval du commmit pour des raisons d'efficacité. En amont quand on travail par patch pour forcer les contributeurs à répondre à des métriques objectives qui viennent en complément de la revue de code subjective par les développeurs. En aval dans les autres cas, ou on vérifie l'intégrité entière du repository (compilation, checkers, test unitaires/fonctionels/intégration etc.).

              Mon commentaire indique aussi que la qualité est une démarche. Utiliser des barrières trop fermes dans cet objectif peut nuire à l'objectif. Tu perds en productivité et en souplesse mais il te faut toujours le même souhait de qualité en plus du hook. Avoir un système de métrique qualité (souvent dans la partie intégration continue) est souvent suffisant. L'objectif affiché est que ce soit toujours vert mais on ne tombe pas dans l'excès inverse de ne jamais permettre le rouge. Par contre c'est la responsabilité de l'équipe que quand c'est rouge ça ne le reste pas. De toute facon si l'équipe n'applique pas ça tu vas dans le mur quoi qu'il arrive.

              En gros en général on est souple au développement et ferme à la livraison. C'est à dire qu'on s'autorise des petites instabilité pour développer mais qu'à la livraison c'est toujours nickel. D'où l’intérêt de faire de petits incréments.

              est largement compensé par le temps économisé (attente que le contributeur revienne pour qu'il ajoute le fichier manquant, ou revert de ses derniers commits)

              Pour les rares fois ou ça t'impacte, le mec qui commit et se tire sans vérifier le build, un rollback local ça prend 10s. Et tu le feras de toute façon si il a introduit une régression qui t'impacte.

  • # g++ -c stubs.c : ça marche !

    Posté par . Évalué à -6.

    Je pense que g++ sait compiler un fichier stubs.c, voir même un fichier stubs.toto
    C'est quoi ce système de compilation alakon ??
    Qu'il y ait des règles par défaut, c'est très bien. Par contre, qu'on ne puisse pas les surcharger quand on le souhaite, c'est vraiment pourri.

    Ah, et sinon, c++, c'est bon et sans huile de palme, mangez-en !

    • [^] # Re: g++ -c stubs.c : ça marche !

      Posté par (page perso) . Évalué à 4.

      C'est quoi ce système de compilation alakon ??

      c'est des Makefiles avec des regles de compilation qui utilisent gcc pour les fichiers .c et g++ pour les fichiers .cpp

      • [^] # Re: g++ -c stubs.c : ça marche !

        Posté par . Évalué à -5.

        Les règles par défaut, c'est très pratique !
        Mais ne pas pouvoir simplement en surcharger une pour un cas particulier, c'est pas terrible.

        (et de deux…)

        • [^] # Re: g++ -c stubs.c : ça marche !

          Posté par . Évalué à 1.

          Sauf que si c'est possible :

          CC=g++ make
          
          

          Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

          • [^] # Re: g++ -c stubs.c : ça marche !

            Posté par . Évalué à -6.

            Les règles par défaut, c'est trop kikoolol \o/
            Mais ne pas pouvoir simplement en surcharger une pour un cas particulier, c'est pas gloglop /o\

            (et de trois… je dois avoir un accent, je ne vois pas d'autres explications)

            • [^] # Re: g++ -c stubs.c : ça marche !

              Posté par . Évalué à 2.

              Désolé, j'ai mal lu j'ai surtout voulu montrer que même si on ne peut peut être pas sur charger les règles, on peut tout de même les influencer pour correspondre à ce dont parle le journal.

              Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

            • [^] # Re: g++ -c stubs.c : ça marche !

              Posté par (page perso) . Évalué à 3.

              Sauf que si c'est possible:

              bionic/stubs.o: bionic/stubs.c
                      $(CXX) -c $^ -o $@
              
              

              Le problème c'est qu'il ne faut pas oublier de rajouter les différents flags, donc au minimum ça donnerait ça:

              bionic/stubs.o: bionic/stubs.c
                      $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $^ -o $@
              
              

              Et si jamais il y a d'autres flags à rajouter ça peut poser marcher chez soi mais pas chez d'autres juste pour ce fichier…

              Donc au final avec les makefiles de base on a les deux possibilités :

              • utiliser des règles par défaut qui sont communes pour tout le monde
              • et/ou surcharger pour certains fichiers mais en prenant le risque d'avoir justement des différences avec le reste

              Une première alternative serait de définir deux extensions différentes ".oxx" (ou ".opp") et ".o", et avoir 3 règles génériques :

              • .c -> .o (avec CC, normal)
              • .cpp -> .oxx (avec CXX, normal)
              • .c -> .oxx (avec CXX, ce qui est demandé ici)
              • la quatrième règle .cpp -> .o n'est normalement pas possible, donc le mieux est de ne pas la définir pour forcer les développeurs à renommer le .cpp en .c si c'est bien du C

              Ensuite dans le makefile on peut indiquer facilement si on veut utiliser CC ou CXX grâce à l'extension ".o" ou ".oxx" quand on mentionne les dépendances.

              Une autre possibilité serait d'avoir des macros pour compiler avec CC ou avec CXX, mais c'est beaucoup moins élégant.

              Tout ça demande des modifications des Makefiles pour un cas quand même assez rare.

              Make est assez bas niveau pour que quasiment tout soit possible (au prix d'un effort plus ou moins important).

              • [^] # Re: g++ -c stubs.c : ça marche !

                Posté par . Évalué à 2.

                Une autre possibilité serait d'avoir des macros pour compiler avec CC ou avec CXX, mais c'est beaucoup moins élégant.

                Tu veut dire que la solution que je présente plus haut n'est pas élégante ? Je m'en sert souvent (pas pour redéfinir CC, mais pour redéfinir LDFLAGS et/ou CFLAGS quand j'utilise make sans makefile).

                Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

  • # Pas la même sémantique

    Posté par (page perso) . Évalué à 9.

    Le C et le C++ n'ont pas exactement la même sémantique, même sur leur sous-ensemble commun. Compiler l'un comme l'autre, c'est chercher la merde là où elle n'est pas.

    • [^] # Re: Pas la même sémantique

      Posté par . Évalué à 4.

      Tu penses à quoi ?

      • [^] # Re: Pas la même sémantique

        Posté par (page perso) . Évalué à 5.

        sizeof('c') par exemple.
        Wikipedia peut donner d'autres idées : http://en.wikipedia.org/wiki/Compatibility_of_C_and_C%2B%2B

        • [^] # Re: Pas la même sémantique

          Posté par . Évalué à 2.

          Mouais il faut avoir une bonne raison pour utiliser intentionnellement pas mal des subtilités listées dans l'article. Il y a quelques trucs importants mais le sizeof('c') par exemple je trouve pas de bonnes raisons de l'utiliser à brûle pourpoint. Quand ce ne sont pas des trucs susceptibles de rendre explicites des implicites, ce qui n'est pas forcément un mal.

          Ça peut être largement balancé si le compilateur C++ est largement plus malin que le compilateur C, ce qui pourrait par contre être assez étonnant.

          • [^] # Re: Pas la même sémantique

            Posté par (page perso) . Évalué à 1.

            Ça peut être largement balancé si le compilateur C++ est largement plus malin que le compilateur C, ce qui pourrait par contre être assez étonnant.

            g++ est beaucoup moins permissif sur les conversion de type implicites crades que gcc C et c'est déja un plus.

            En contre-parti, il y a des choses que GCC C supporte et pas g++ comme les nested function et les allocations const de structure imbriquées entre autres.

            • [^] # Re: Pas la même sémantique

              Posté par . Évalué à 2.

              Je vais peut-être dire une bêtise, mais c'est pas un peu lourd en C de devoir écrire :
              int *a = (int *) malloc(10*sizeof(int));
              au lieu de int *a = malloc(10*sizeof(int));
              à cause de la compilation en C++ ?

              En C++ on a new donc malloc et ses amis peuvent être évités, mais en C on n'a pas trop le choix…

    • [^] # Re: Pas la même sémantique

      Posté par . Évalué à 5.

      Quand je le faisais, je n'utilisais pas le code généré par g++. Une fois que je n'avais plus de warning g++, je compilais en gcc et j'utilisais ce dernier.

      Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

  • # Ouai

    Posté par . Évalué à 7.

    Ça m'est déjà arrivé, mais aujourd'hui c'est plutôt vers de l'analyse statique de code que je me tournerais pour avoir le même résultat en mieux et plus propre.

    Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

  • # Tant qu'à faire

    Posté par (page perso) . Évalué à 8.

    Tant qu'à faire, autant écrire du code en C++.

  • # Lint

    Posté par . Évalué à 2.

    Perso, j'ai découvert récemment splint (ou lint).

    C'est un vérificateur de code. Mon code compilait sans aucun warning. Mais une fois lancé, j'ai eu droit a des kilomètres de warning avec ça. Et pour la plupart c'était justifié.

    Le seul inconvénient, c'est que pour supprimer certain warning il faut annoter son code.

Suivre le flux des commentaires

Note : les commentaires appartiennent à ceux qui les ont postés. Nous n'en sommes pas responsables.