Umgang mit Warnungen in einem Legacy-Projekt

9

Ich arbeite an einem C ++ - Projekt, das unzählige Warnungen generiert. Die meisten Warnungen wurden angezeigt, nachdem der Code geschrieben wurde:

  • Ursprünglich verwendete das Projekt Visual C ++ 8 und wechselte bald zu 9, aber die generierten Warnungen unterscheiden sich kaum. Die Warnungen wurden entweder behoben oder zum Schweigen gebracht, sodass es damals keine Warnungen gab.
  • Dann wurde ein 64-Bit-Ziel hinzugefügt. Es wurden eine große Anzahl von Warnungen generiert, hauptsächlich aufgrund der schlampigen Verwendung von Typen (z . B. unsignedvs. size_t). Niemand machte sich die Mühe oder hatte Zeit, sie zu reparieren, sobald der Code für diesen Zweck funktionierte.
  • Dann wurde die Unterstützung für andere Plattformen mit GCC (4.5 und 4.6, anfangs einige 4.4) hinzugefügt. GCC ist viel wählerischer, daher wurden viel mehr Warnungen generiert. Wieder machte sich niemand die Mühe oder hatte Zeit, sie zu reparieren. Dies wurde durch die Tatsache erschwert, dass GCC bis 4.5 kein Pragma hatte, um eine Warnung in einem bestimmten Code-Bit zum Schweigen zu bringen, und laut Dokumentation ist es immer noch nicht das, was man brauchen würde.
  • In der Zwischenzeit tauchten einige veraltete Warnungen auf.

Jetzt haben wir ein Projekt, das Tausende von Warnungen generiert. Und ich kann nicht einmal sagen, an wie vielen Stellen, da selbst .cppDateien mehrmals von verschiedenen Compilern kompiliert werden und Warnungen in Headern immer wieder gedruckt werden.

Gibt es eine bewährte Methode, um so etwas zu bereinigen? Oder zumindest einige positive Erfahrungen damit?

Jan Hudec
quelle

Antworten:

4

Berühren Sie keinen der Legacy-Codes. Finden Sie einen Weg, um die Warnungen vorerst zu unterdrücken. Das Vornehmen scheinbar harmloser Änderungen am Code kann zu Fehlern im Code führen, der, wie ich annehme, bereits getestet wurde und relativ fehlerfrei ist.

Indem Sie alle Warnungen unterdrücken, die sich derzeit in der Codebasis befinden, können Sie so tun, als hätten Sie eine saubere Tafel. Alle neuen Warnungen, die in den Code eingefügt werden, sollten entfernt werden. Wenn der alte Code neu geschrieben wird, sollte die neue Implementierung keine Warnungen unterdrücken.

Ihre Ziele hier sollten sein:

  1. Reduzieren Sie das Signal / Rausch-Verhältnis. Wenn Entwickler beim Erstellen Tausende von Benachrichtigungen sehen, werden sie nicht bemerken, dass der Hauptzweig 2004 Benachrichtigungen und ihr Zweig 2006 hat.

  2. Führe keine Bugs mehr ein. Wenn Sie die oben genannten harmlosen Änderungen vornehmen, können Sie unvorhergesehene Fehler verursachen. Dies ist besonders relevant, wenn Sie Tausende von Warnungen haben. Selbst wenn Ihre Fehlerrate unglaublich niedrig ist, haben Sie immer noch Tausende von Chancen, Fehler zu machen.

  3. Lassen Sie nicht zu, dass der Legacy-Code Ihre Standards für neuen Code senkt. Nur weil die Warnungen für eine Klasse unterdrückt wurden, bedeutet dies nicht, dass die neue Klasse dasselbe tun kann. Mit Sorgfalt wird der Legacy-Code schließlich aus der Codebasis entfernt und durch neueren, warnungsfreien Code ersetzt.

Jonathan Rich
quelle
1
+1 "Das Vornehmen scheinbar harmloser Änderungen am Code kann zu Fehlern im Code führen ..." - Legacy-Code wird wahrscheinlich nicht automatisch getestet. Diese Fehler werden nicht gefunden.
Mattnz
@mattnz: Tatsächlich sind die kleinen automatisierten Tests sowohl auf Legacy- als auch auf Nicht-Legacy-Code verteilt. Es ist eine interaktive Anwendung, die größtenteils nicht angemessen getestet werden kann. Die vorhandenen Komponententests stürzen (bei Speicherbeschädigung) auf dem Continuous Integration Server ab. Seit über einem Jahr, aber niemand hat sich jemals darum gekümmert, das Problem zu beheben, da es unter dem Debugger nicht reproduziert werden kann. Und um nützlich zu sein, müsste der CI-Server einige Datenbeispiele zum Testen abrufen, auf denen auch nie implementiert wurde.
Jan Hudec
1
Das läuft darauf hinaus, wie die vorhandenen Warnungen effizient stummgeschaltet werden können, ohne dass ähnliche Warnungen im neuen Code stummgeschaltet werden.
Jan Hudec
@ Jan: Wollen Sie wirklich sagen, dass Sie sich nicht die Mühe machen müssen, die wenigen Tests zu reparieren, die Sie haben? In einem Programmierforum? "Ja wirklich?" (SMH)
Mattnz
2
@ Mattnz: Ja. Weil die Tests nutzlos sind. Und wird es immer sein. Im häufigsten Fall von Geschäftsinformationssystemen sind automatisierte Tests großartig, aber für die Art der interaktiven Anwendung, die wir haben, bringen sie so wenig Wert, dass wir sie nur gelegentlich zum Debuggen einiger Supportfunktionen verwenden. Die meisten Fehler treten in Code auf, der nur interaktiv getestet werden kann. Ich kann mir vorstellen, etwas Automatisierung dafür zu schreiben, aber es wäre eine Menge Arbeit, von der ich nicht sicher bin, ob sie kosteneffizient wäre.
Jan Hudec
14

Ich halte Warnungen, die nicht als Fehler behandelt werden, für nutzlos. Man bekommt Tonnen von ihnen, deshalb stört es niemanden, sie anzusehen, und sie verfehlen ihren Zweck.

Mein Vorschlag ist, sie alle zu beheben und sie als Fehler zu behandeln.
Sie zu reparieren sieht beängstigend aus, aber ich denke, es kann getan werden. Ein guter Programmierer, der diese Arbeit aufgreifen würde, wird sehr bald herausfinden, wie mit jeder Warnung umgegangen werden soll, und wird sehr mechanisch und schnell damit umgehen.

Wir haben ein ähnliches Projekt in meiner Firma durchgeführt und es hat funktioniert - wir haben jetzt keine Warnungen in dem Teil des Codes, in dem es für uns wichtig ist, und ich spreche über viele tausend Dateien (ich weiß nicht, wie viele Warnungen) .

Daraufhin müssen Warnungen sofort als Fehler behandelt werden. Sobald ein Teil Ihres Codes warnungsfrei ist, ändern Sie die Kompilierungsflags für diesen Teil und lassen Sie keine neuen Warnungen eintreten. Wenn Sie dies nicht tun, werden neue Warnungen wie Pilze angezeigt, und Ihre Arbeit wird verschwendet.

Ein Problem, das Sie möglicherweise haben, ist die Einführung von Fehlern infolge der Behebung der Warnungen. Dies ist besonders wahrscheinlich, weil der Programmierer, der die Warnungen korrigiert, den größten Teil des Codes, den er ändert, wahrscheinlich nicht kennt. Die meisten Änderungen wären jedoch sehr sicher - z. B. das Hinzufügen eines Funktionsprototyps oder einer Besetzung.

ugoren
quelle
2
+1: "Ich denke, Warnungen, die nicht als Fehler behandelt werden, sind nutzlos.": Ich stimme zu, warum sollte ich vor etwas gewarnt werden wollen, das kein Problem darstellt?
Giorgio
1
@Giorgio Weil Warnungen nicht unbedingt behoben werden müssen oder möglicherweise nicht genügend Zeit / Geld zur Verfügung steht, um sie zu beheben. Sie sind eine Form der technischen Verschuldung. Das bedeutet nicht, dass sie nicht irgendwann gehandhabt werden sollten, aber es bedeutet auch, dass sie nicht unbedingt unter den Teppich gekehrt werden sollten. Ihre einzige verbleibende Option ist, sie zu haben; Sie sind da, um dich zu jucken, damit sie vielleicht irgendwann repariert werden.
Robert Harvey
1
@ Robert Harvey: Ich stimme dir vollkommen zu. Das einzige Risiko besteht darin, dass man anfängt, Warnungen zu ignorieren, und nach einer Weile juckt es Sie nicht mehr: Ich habe jahrelang Warnungen im selben Projekt gesehen, und jeder Entwickler würde sagen, dass sie "irgendwann" behoben werden sollten die Zukunft". Niemand achtete mehr auf diese Warnungen: Sie waren Teil der normalen Ausgabe im Master-Build-Protokoll geworden. Ich bin mir nicht sicher, wie ich solche Situationen vermeiden soll.
Giorgio
1
Das Problem bei der Behebung aller "Jetzt" -Mentalitäten besteht darin, dass sie zu falschen Korrekturen und weniger wartbarem Code führen. In C ++ ist es erstaunlich, wie leicht eine Typumwandlung eine Warnung verschwinden lassen kann, meistens das Falsche (z. B. signieren / nicht signiert) Warnungen). Es ist so unwahrscheinlich, dass Legacy-Code automatisierte Regressionstests durchführt, und Codeänderungen führen zu Fehlern. "Fix em all und b.gg.r die Konsequenzen" ist nicht der richtige Ansatz.
Mattnz
@mattnz, Das Brechen des Codes beim Korrigieren der Warnung ist in der Tat das Hauptanliegen hier. Ich denke jedoch, dass es keinen anderen Weg gibt - entweder sie zu beheben und zukünftige als Fehler zu behandeln oder sie zu ignorieren (und gelegentlich Fehler zu finden, vor denen Sie gewarnt wurden).
Ugoren
3

Ich habe C ++ - Codierungsstandards gelesen : 101 Regeln, Richtlinien und bewährte Methoden, und die zweite der Richtlinien schlägt vor: "Kompilieren Sie sauber bei hohen Warnstufen". Es gibt eine Reihe von Gründen, warum Sie nicht möchten, dass so viele Warnungen in einem Programm und Chef unter ihnen sind

  1. Es gibt ein potenzielles Problem in Ihrem Code.
  2. Sollte stille, erfolgreiche Builds haben und wenn nicht, könnten Ihnen echte Probleme fehlen.
  3. Ignorierte gutartige Warnungen können Warnungen verdecken, die auf echte Gefahren hinweisen

Programme, die mit Warnungen kompiliert werden, erscheinen möglicherweise korrekt, aber es ist möglich, dass es echte Probleme mit dem Code gibt, die irgendwann auftreten können. Ich würde vorschlagen, die Warnungen zu kategorisieren, ihnen eine Priorisierungsstufe zu geben und sie zu korrigieren, wenn Sie Zeit haben.

Matschig
quelle
+1: für "Compile Cleany bei hohen Warnstufen": Ich denke, wir sollten so viel Hilfe wie möglich vom Compiler verwenden.
Giorgio
1

Legen Sie entweder einen Sprint / Zyklus beiseite, um vorbeugende Wartungsarbeiten am System durchzuführen, toten Code zu löschen, Warnungen zu beseitigen, schlechte Kommentare zu bereinigen, oder eine Richtlinie, um Warnungen zu beseitigen, da Quellen ohnehin für etwas anderes berührt werden.
Ersteres würde ich nicht selbst tun, um Warnungen loszuwerden, aber wenn ein solcher Zyklus geplant wäre, wäre das Teil der Arbeitsbelastung. Der Grund dafür ist, dass das Berühren einer Datei für irgendetwas das Risiko von Fehlern birgt und somit die Stabilität Ihres Produkts gefährdet.

jwenting
quelle
2
Wir können es uns nicht leisten, einen Sprint beiseite zu legen. Beseitigen Sie also Warnungen, wenn Quellen aus anderen Gründen berührt werden. Es gibt jedoch einige Quellen, die lange Zeit nicht berührt wurden, und es wird lange dauern, bis sie es tun. Daher müssen wir die Warnungen von diesen alten Teilen unterscheiden.
Jan Hudec
@JanHudec möchte in diesen Fällen möglicherweise eine Analyse des Schweregrads der Warnungen durchführen. Planen Sie möglicherweise eine gewisse Zeit ein, um den alten Code erneut zu überprüfen und ausgewählte Bits zu bereinigen. Refactor, um ihn am Leben zu erhalten. Andernfalls kann es abgestanden werden und die Leute haben Angst, es zu berühren, selbst wenn es gebraucht wird, weil niemand mehr weiß, wie es funktioniert.
Jwenting
@JanHudec - Klingt so, als hätten Sie keine Zeit, die Warnungen zu beheben, wenn Sie es sich nicht leisten können, vorbeugende Wartungsarbeiten am System durchzuführen.
Ramhound
@ Ramhound: Ich nehme an, wir könnten ein bisschen Hausmeister machen, aber nicht viel und es muss parallel zu anderen Arbeiten laufen.
Jan Hudec
1

Ich fühle deinen Schmerz, weil ich in einer ähnlichen Situation bin. Die Art und Weise, wie ich mich dem Problem näherte, ist: Entfernen Sie die Warnungen jeweils für ein Modul / Teilprojekt und setzen Sie das Werr-Compiler-Flag "Warnungen als Fehler behandeln" (Werr) für das Modul, nachdem es bereinigt wurde. Außerdem habe ich beschlossen, mich nur auf eine Plattform zu konzentrieren, obwohl wir auf mehreren Betriebssystemen mit unterschiedlichen Compilern aufbauen. Schließlich schalte ich für die Bibliotheken des dritten Teils, die wir kompilieren, einfach die Warnungen aus.

Wohlgemerkt, einige dieser Warnungen loszuwerden ist viel einfacher gesagt als getan. Beispielsweise können von Ihnen erwähnte Probleme mit size_t im Vergleich zu nicht signierten Problemen zu ganzzahligen Überläufen führen, wenn Sie die Warnungen nur durch Casts stummschalten. In einigen Fällen ersetzen Sie unsigned durch size_t, in anderen Fällen müssen Sie zur Laufzeit einen Überlauf feststellen. Es ist alles von Fall zu Fall - sehr langweilig und hart.

Nemanja Trifunovic
quelle