Es ist schlecht, viele Aktionen in einem Ausdruck der C ++ - Sprache zu kombinieren, da ein solcher Code schwer zu verstehen, schwer zu pflegen und auch leicht fehlerhaft ist. Erstellen Sie beispielsweise einen Fehler, indem Sie beim Auswerten von Funktionsargumenten verschiedene Aktionen kombinieren. Wir stimmen der klassischen Empfehlung zu, dass der Code einfach und unkompliziert sein sollte. Und jetzt werden wir einen interessanten Fall betrachten, in dem der PVS-Studio-Analysator formal falsch ist, aber aus praktischer Sicht der Code noch geändert werden sollte.
Reihenfolge der Argumentauswertung
Was jetzt erzählt wird, ist eine Fortsetzung der alten Geschichte über die Reihenfolge der Berechnung von Argumenten, über die wir im Artikel " Die Tiefe eines Kaninchenlochs oder ein Interview in C ++ bei PVS-Studio " geschrieben haben.
Das kurze Wesen ist wie folgt. Die Reihenfolge, in der Funktionsargumente ausgewertet werden, ist nicht angegebenes Verhalten. Der Standard gibt nicht die genaue Reihenfolge an, in der Compilerentwickler die Argumente auswerten müssen. Zum Beispiel von links nach rechts (Clang) oder von rechts nach links (GCC, MSVC). Vor dem C ++ 17-Standard konnte dies zu undefiniertem Verhalten führen, wenn bei der Bewertung von Argumenten Nebenwirkungen auftraten.
Mit dem Aufkommen des C ++ 17-Standards hat sich die Situation zum Besseren gewendet: Jetzt wird die Berechnung des Arguments und seiner Nebenwirkungen erst ab dem Moment durchgeführt, an dem alle Berechnungen und Nebenwirkungen des vorherigen Arguments durchgeführt wurden. Dies bedeutet jedoch nicht, dass jetzt kein Raum für Fehler vorhanden ist.
Betrachten Sie ein einfaches Testprogramm:
#include <cstdio>
int main()
{
int i = 1;
printf("%d, %d\n", i, i++);
return 0;
}
Was wird dieser Code drucken? Die Antwort hängt immer noch vom Compiler, der Version und der Stimmung ab. Je nach Compiler können sowohl "1, 1" als auch "2, 1" gedruckt werden. In der Tat erhalte ich mit dem Compiler-Explorer die folgenden Ergebnisse:
- Ein mit Clang 11.0.0 kompiliertes Programm erzeugt "1, 1".
- Ein mit GCC 10.2 kompiliertes Programm erzeugt "2, 1".
Es gibt kein undefiniertes Verhalten in diesem Programm, aber es gibt ein nicht spezifiziertes Verhalten (Reihenfolge der Bewertung von Argumenten).
Code aus dem CSV Parser-Projekt
Kehren wir zum Code-Snippet aus dem CSV-Parser-Projekt zurück, das ich im Artikel " Überprüfen der Sammlung von C ++ - Bibliotheken nur für Header (awesome-hpp) " erwähnt habe.
Der Parser und ich wissen, dass Argumente in einer anderen Reihenfolge ausgewertet werden können. Daher betrachtete der Analysator und danach ich diesen Code als fehlerhaft:
std::unique_ptr<char[]> buffer(new char[BUFFER_UPPER_LIMIT]);
....
this->feed_state->feed_buffer.push_back(
std::make_pair<>(std::move(buffer), line_buffer - buffer.get()));
PVS-Studio- Warnung : V769 Der Zeiger 'buffer.get ()' im Ausdruck 'line_buffer - buffer.get ()' entspricht nullptr. Der resultierende Wert ist sinnlos und sollte nicht verwendet werden. csv.hpp 4957
Eigentlich liegen wir beide falsch und es gibt keinen Fehler. Wir werden weiter über die Nuancen sprechen, aber jetzt beginnen wir mit einer einfachen.
Mal sehen, warum es gefährlich ist, Code wie diesen zu schreiben:
Foo(std::move(buffer), line_buffer - buffer.get());
Ich denke, Sie können die Antwort erraten. Das Ergebnis hängt von der Reihenfolge ab, in der die Argumente ausgewertet werden. Berücksichtigen Sie dies im folgenden synthetischen Code:
#include <iostream>
#include <memory>
void Print(std::unique_ptr<char[]> p, ptrdiff_t diff)
{
std::cout << diff << std::endl;
}
void Print2(ptrdiff_t diff, std::unique_ptr<char[]> p)
{
std::cout << diff << std::endl;
}
int main()
{
{
std::unique_ptr<char[]> buffer(new char[100]);
char *ptr = buffer.get() + 22;
Print(std::move(buffer), ptr - buffer.get());
}
{
std::unique_ptr<char[]> buffer(new char[100]);
char *ptr = buffer.get() + 22;
Print2(ptr - buffer.get(), std::move(buffer));
}
return 0;
}
Lassen Sie uns den Compiler-Explorer erneut verwenden und die Ausgabe dieses Programms sehen, die von verschiedenen Compilern kompiliert wurde.
Clang Compiler 11.0.0. Ergebnis :
23387846
22
Der GCC-Compiler 10.2. Ergebnis :
22
26640070
Wir erwarten das Ergebnis, und so kann man nicht schreiben. Genau davor warnt der PVS-Studio-Analysator.
Ich würde dem gerne ein Ende setzen, aber alles ist etwas komplizierter. Tatsache ist, dass es sich um die Übergabe von Argumenten nach Wert handelt. Wenn Sie die Funktionsvorlage std :: make_pair instanziieren , ist alles anders. Lassen Sie uns weiter in die Nuancen eintauchen und herausfinden, warum PVS-Studio in diesem Fall falsch ist.
std :: make_pair
Werfen wir einen Blick auf die cppreference-Website und sehen, wie sich die Funktionsvorlage std :: make_pair geändert hat .
Bis C ++ 11:
Vorlage <Klasse T1, Klasse T2>Seit C ++ 11 bis C ++ 14:
std :: pair <T1, T2> make_pair (T1 t, T2 u);
Vorlage <Klasse T1, Klasse T2>Seit C ++ 14:
std :: pair <V1, V2> make_pair (T1 && t, T2 && u);
template< class T1, class T2 >Wie Sie sehen, hat std :: make_pair einst Argumente nach Wert akzeptiert. Wenn zu diesem Zeitpunkt std :: unique_ptr vorhanden wäre, wäre der obige Code tatsächlich falsch. Ob dieser Code funktionieren würde oder nicht, war eine Frage des Glücks. In der Praxis wäre diese Situation natürlich nie aufgetreten, da std :: unique_ptr in C ++ 11 als Ersatz für std :: auto_ptr erschien .
constexpr std::pair<V1,V2> make_pair( T1&& t, T2&& u );
Gehen wir zurück zu unserer Zeit. Beginnend mit der Version des C ++ 11-Standards begann der Konstruktor mit der Verwendung der Verschiebungssemantik.
Es gibt hier einen subtilen Punkt darin, dass std :: move eigentlich nichts verschiebt, sondern das Objekt nur in eine rvalue- Referenz konvertiert . Dies wird ermöglichenstd :: make_pair übergibt den Zeiger an den neuen std :: unique_ptr , wobei nullptr im ursprünglichen intelligenten Zeiger verbleibt . Aber diese Zeigerübergabe wird erst passieren, wenn wir in std :: make_pair sind . Zu diesem Zeitpunkt haben wir bereits line_buffer - buffer.get () berechnet , und alles wird gut. Das heißt, ein Aufruf von buffer.get () kann zum Zeitpunkt der Auswertung kein nullptr zurückgeben , unabhängig davon, wann dies geschieht.
Entschuldigung für die komplizierte Beschreibung. Die Quintessenz ist, dass dieser Code vollkommen gültig ist. Tatsächlich gab der statische Analysator PVS-Studio in diesem Fall einen Fehlalarm aus. Unser Team ist sich jedoch nicht sicher, ob wir uns beeilen sollten, Änderungen an der Logik des Analysators für solche Situationen vorzunehmen.
Der König ist tot, es lebe der König!
Wir haben herausgefunden, dass sich die im Artikel beschriebene Operation als falsch herausgestellt hat. Vielen Dank an einen unserer Leser, der uns auf die Besonderheit der Implementierung von std :: make_pair aufmerksam gemacht hat .
Dies ist jedoch der Fall, wenn wir nicht sicher sind, ob es sich lohnt, das Verhalten des Analysators zu verbessern. Der Punkt ist, dass dieser Code zu kompliziert ist. Sie müssen zugeben, dass der von uns analysierte Code keine so detaillierte Untersuchung verdient, die einen ganzen Artikel in Mitleidenschaft gezogen hat. Wenn dieser Code so viel Aufmerksamkeit erfordert, ist er ein sehr schlechter Code.
Es ist hier angebracht, an den Artikel " False Positives sind unsere Feinde, aber möglicherweise immer noch Ihre Freunde " zu erinnern . Die Veröffentlichung gehört nicht uns, aber wir stimmen ihr zu.
Dies ist vielleicht genau der Fall. Die Warnung mag falsch sein, weist jedoch auf einen besseren Ort für die Umgestaltung hin. Es reicht aus, so etwas zu schreiben:
auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.push_back(
std::make_pair(std::move(buffer), delta));
Oder Sie können den Code in dieser Situation mithilfe der emplace_back- Methode noch verbessern :
auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.emplace_back(std::move(buffer), delta);
Dieser Code erstellt das resultierende std :: pair- Objekt im Container "an Ort und Stelle", wobei die Erstellung eines temporären Objekts umgangen und in den Container verschoben wird. Übrigens schlägt der PVS-Studio-Analysator vor, einen solchen Ersatz vorzunehmen, indem eine Warnung V823 aus dem Regelwerk für Code- Mikrooptimierungen ausgegeben wird .
Der Code wird für jeden Leser und Analysator eindeutig einfacher und klarer. Es ist nicht sinnvoll, so viele Aktionen wie möglich in eine Codezeile zu packen.
Ja, in diesem Fall hatte es Glück und es gibt keinen Fehler. Es ist jedoch unwahrscheinlich, dass der Autor beim Schreiben dieses Codes alles, was wir besprochen haben, in seinem Kopf behalten hat. Höchstwahrscheinlich war es das Glück, das gespielt hat. Und ein anderes Mal haben Sie vielleicht kein Glück.
Fazit
Wir haben also herausgefunden, dass es keinen wirklichen Fehler gibt. Der Analysator erzeugt ein falsches Positiv. Vielleicht entfernen wir die Warnung nur für solche Fälle oder vielleicht auch nicht. Wir werden später darüber nachdenken. Immerhin ist dies ein eher seltener Fall, und Code, bei dem Argumente mit Nebenwirkungen bewertet werden, ist im Allgemeinen gefährlich, und es ist besser, ihn zu vermeiden. Zumindest zu vorbeugenden Zwecken lohnt es sich, eine Umgestaltung vorzunehmen.
Code anzeigen:
Foo(std::move(buffer), line_buffer - buffer.get());
leicht zu brechen, indem man etwas an anderer Stelle im Programm ändert. Code wie dieser ist schwer zu pflegen. Und es ist auch insofern unangenehm, als es ein falsches Gefühl vermitteln kann, dass alles richtig funktioniert. Tatsächlich ist dies nur ein Zufall, und alles kann kaputt gehen, wenn Sie die Compiler- oder Optimierungseinstellungen ändern.
Machen Sie Ihren Code einfacher!
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Andrey Karpov. Der Code Analyzer ist falsch. Es lebe der Analysator! ...