Kürzlich hat ABBYY den Quellcode für sein NeoML-Framework veröffentlicht. Es wurde uns angeboten, diese Bibliothek mit PVS-Studio zu überprüfen. Dies ist aus analytischer Sicht ein interessantes Projekt, daher haben wir es nicht auf den Prüfstand gestellt. Sie werden nicht viel Zeit brauchen, um diesen Artikel zu lesen, da das Projekt von hoher Qualität ist :).
Der NeoML- Quellcode ist auf GitHub zu finden . Dieses Framework ist plattformübergreifend und dient zur Implementierung von Modellen für maschinelles Lernen. Es wird von ABBYY-Entwicklern verwendet, um Probleme mit der Bildverarbeitung, der Verarbeitung natürlicher Sprache, einschließlich Bildverarbeitung, Dokumentenanalyse usw. zu lösen. Derzeit werden Programmiersprachen wie C ++, Java, Objective-C unterstützt, und Python sollte bald zu dieser Liste hinzugefügt werden. Die Hauptsprache, in der das Framework selbst geschrieben wurde, ist C ++.
Analyse ausführen
Es war sehr einfach, die Analyse in diesem Framework durchzuführen. Nachdem ich ein Visual Studio-Projekt in CMake generiert hatte, startete ich die PVS-Studio-Analyse in Visual Studio für die Projekte, die für uns von Solution von Interesse sind, ausgenommen Bibliotheken von Drittanbietern. Neben NeoML selbst enthielt die Lösung auch Bibliotheken von ABBYY wie NeoOnnx und NeoMathEngine. Ich habe sie auch in die Liste der Projekte aufgenommen, für die die Analyse gestartet wurde.
Analyseergebnisse
Natürlich wollte ich wirklich einige schreckliche Fehler finden, aber ... der Code erwies sich als ziemlich sauber und es wurden nur Warnungen empfangen, nichts. Es ist wahrscheinlich, dass in der Entwicklung bereits statische Analysen verwendet wurden. Viele der Warnungen waren Auslöser derselben Diagnose für ähnliche Abschnitte des Codes.
In diesem Projekt wird beispielsweise häufig eine virtuelle Methode von einem Konstruktor aufgerufen. Im Allgemeinen ist dies ein gefährlicher Ansatz. Diagnose V1053 reagiert auf solche Fälle : Das Aufrufen der virtuellen Funktion 'foo' im Konstruktor / Destruktor kann zur Laufzeit zu unerwarteten Ergebnissen führen . Insgesamt wurden 10 solcher Warnungen ausgegeben. Lesen Sie in diesem Artikel von Scott Myers mehr darüber, warum dies eine gefährliche Praxis ist und welche Probleme sie verursacht. "Rufen Sie niemals virtuelle Funktionen während der Konstruktion oder Zerstörung auf . "Anscheinend verstehen die Entwickler jedoch, was sie tun, und es gibt keine Fehler.
Es gibt auch 11 V803- Diagnosewarnungen aus dem Abschnitt Mikrooptimierungen . In dieser Diagnose wird empfohlen, das Postfix-Inkrement durch ein Präfix zu ersetzen. Wenn der vorherige Wert des Iterators nicht verwendet wird. Im Fall eines Postfix-Inkrements wird ein zusätzliches temporäres Objekt erstellt. Dies ist natürlich kein Fehler, sondern nur ein kleines Detail . Wenn eine solche Diagnose nicht interessant ist, können Sie sie bei Verwendung des Analysators einfach ausschalten. Nun, im Prinzip eine Reihe von "Mikro-" Optimierungen "und standardmäßig deaktiviert.
Eigentlich denke ich, dass Sie verstehen, dass, da wir zu der Analyse im Artikel über Kleinigkeiten wie das Inkrement des Iterators gekommen sind, im Allgemeinen alles in Ordnung ist und wir einfach nicht wissen, woran wir etwas auszusetzen haben.
Sehr oft sind einige Diagnosen für den Benutzer nicht anwendbar oder uninteressant, und es ist besser, keinen Kaktus zu essen, sondern ein wenig Zeit damit zu verbringen, den Analysator einzurichten. Weitere Informationen zu den Schritten, die unternommen werden sollten, um den interessantesten Auslösern des Analysators sofort näher zu kommen, finden Sie in unserem Artikel " Wie können interessante Warnungen des PVS-Studio-Analysators für C- und C ++ - Code schnell angezeigt werden? "
Unter den Auslösern aus dem Abschnitt " Mikrooptimierungen "Es gibt auch interessante V802- DiagnosewarnungenHiermit wird empfohlen, die Strukturfelder in absteigender Reihenfolge der Schriftgrößen anzuordnen, um die Strukturgröße zu verringern.
V802 Auf einer 64-Bit-Plattform kann die Strukturgröße von 24 auf 16 Byte reduziert werden, indem die Felder entsprechend ihrer Größe in absteigender Reihenfolge neu angeordnet werden. HierarchicalClustering.h 31
struct CParam {
TDistanceFunc DistanceType;
double MaxClustersDistance;
int MinClustersCount;
};Durch einfach den Austausch MaxClustersDistance Feld mit dem Doppeltyp und der DistanceType enumerator, können Sie die Strukturgröße von 24 bis 16 Bytes reduzieren.
struct CParam {
double MaxClustersDistance;
int MinClustersCount;
TDistanceFunc DistanceType;
};TDistanceFunc ist eine Aufzählung , daher entspricht ihre Größe int oder weniger. Daher sollte sie an das Ende der Struktur verschoben werden.
Auch dies ist kein Fehler, aber wenn der Benutzer an Mikrooptimierungen interessiert ist oder wenn diese im Prinzip für das Projekt wichtig sind, können Sie mit solchen Analysator-Triggern schnell Orte für mindestens ein primäres Refactoring finden.
Im Allgemeinen ist der gesamte Code sauber und leserlich geschrieben, aber die V807- Diagnose zeigte auf einige Stellen, die etwas optimaler und lesbarer gemacht werden könnten. Lassen Sie mich das auffälligste Beispiel geben:
V807 Leistungsminderung. Erstellen Sie eine Referenz, um zu vermeiden, dass derselbe Ausdruck wiederholt verwendet wird. GradientBoostFullTreeBuilder.cpp 469
Der Aufruf von curLevelStatistics [i] -> ThreadStatistics [j] kann durch einen Aufruf einer separaten Variablen ersetzt werden. Es gibt keinen Aufruf für komplexe Methoden in dieser Kette, daher gibt es hier möglicherweise nicht viel Optimierung, aber der Code wird meiner Meinung nach viel einfacher und kompakter gelesen. Mit der Unterstützung dieses Codes wird außerdem in Zukunft deutlich, dass auf genau diese Indizes zugegriffen werden muss und hier kein Fehler vorliegt. Der Klarheit halber werde ich einen Code mit einem Ersatz für eine Variable geben:
auto threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];
if(threadStatistics.FeatureIndex != NotFound ) {
if( threadStatistics.Criterion > criterion
|| ( .... ))
{
criterion = threadStatistics.Criterion;
curLevelStatistics[i]->FeatureIndex = threadStatistics.FeatureIndex;
curLevelStatistics[i]->Threshold = threadStatistics.Threshold;
curLevelStatistics[i]->LeftStatistics = threadStatistics.LeftStatistics;
curLevelStatistics[i]->RightStatistics = threadStatistics.RightStatistics;
}
}Fazit
Wie Sie sehen können, hat sich die Codebasis dieses Frameworks aus Sicht der statischen Analyse als sehr sauber herausgestellt.
Es versteht sich, dass ein Durchlauf der Analyse eines aktiv entwickelten Projekts die Notwendigkeit einer statischen Analyse nur schwach widerspiegelt, da viele Fehler, insbesondere wenn sie kritisch waren, bereits auf andere Weise erkannt wurden, jedoch viel zeitaufwendiger und ressourcenintensiver. Dieser Punkt wurde im Artikel " Fehler, die die statische Code-Analyse nicht finden kann, weil sie nicht verwendet wird " ausführlicher analysiert .
Aber auch unter Berücksichtigung dieser Tatsache wurden nur wenige Warnungen zu NeoML ausgegeben, und ich möchte die Qualität des Codes in diesem Projekt respektieren, unabhängig davon, ob die Entwickler statische Analysen verwendet haben oder nicht.
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Victoria Khanieva. PVS-Studio Beeindruckt von der Codequalität von ABBYY NeoML .