Menggabungkan banyak tindakan dalam satu ekspresi C ++ adalah buruk, karena kode seperti itu sulit dipahami, sulit dipertahankan, dan juga mudah membuat kesalahan di dalamnya. Misalnya, buat bug dengan menggabungkan tindakan yang berbeda saat mengevaluasi argumen fungsi. Kami setuju dengan rekomendasi klasik bahwa kode tersebut harus sederhana dan lugas. Dan sekarang mari kita pertimbangkan kasus yang menarik, ketika secara formal penganalisis PVS-Studio salah, tetapi dari sudut pandang praktis, kodenya masih harus diubah.
Urutan Evaluasi Argumen
Yang akan kami ceritakan sekarang adalah kelanjutan dari cerita lama tentang urutan perhitungan argumen, yang kami tulis di artikel " Kedalaman lubang kelinci atau wawancara dalam C ++ di PVS-Studio ".
Esensi singkatnya adalah sebagai berikut. Urutan di mana argumen fungsi dievaluasi adalah perilaku yang tidak ditentukan. Standar tidak menentukan urutan yang tepat di mana pengembang kompilator harus mengevaluasi argumen. Misalnya, kiri ke kanan (Clang) atau kanan ke kiri (GCC, MSVC). Sebelum standar C ++ 17, ketika efek samping terjadi saat mengevaluasi argumen, ini dapat menyebabkan perilaku tidak terdefinisi.
Dengan munculnya standar C ++ 17, situasinya telah berubah menjadi lebih baik: sekarang kalkulasi argumen dan efek sampingnya akan mulai dijalankan hanya dari saat semua kalkulasi dan efek samping dari argumen sebelumnya telah dilakukan. Namun, ini tidak berarti bahwa sekarang tidak ada ruang untuk kesalahan.
Pertimbangkan program tes sederhana:
#include <cstdio>
int main()
{
int i = 1;
printf("%d, %d\n", i, i++);
return 0;
}
Apa yang akan dicetak kode ini? Jawabannya masih tergantung pada compiler, version, dan moodnya. Tergantung pada kompilernya, baik "1, 1" dan "2, 1" dapat dicetak. Memang, dengan menggunakan Compiler Explorer, saya mendapatkan hasil sebagai berikut:
- program yang dikompilasi dengan Clang 11.0.0 menghasilkan "1, 1".
- program yang dikompilasi dengan GCC 10.2 menghasilkan "2, 1".
Tidak ada perilaku yang tidak ditentukan dalam program ini, tetapi ada perilaku yang tidak ditentukan (urutan evaluasi argumen).
Kode dari proyek CSV Parser
Mari kita kembali ke cuplikan kode dari proyek CSV Parser, yang saya sebutkan di artikel " Memeriksa kumpulan pustaka C ++ khusus header (awesome-hpp) ".
Pengurai dan saya tahu bahwa argumen dapat dievaluasi dalam urutan yang berbeda. Oleh karena itu, penganalisis, dan setelahnya sendiri, menganggap kode ini salah:
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()));
Peringatan PVS-Studio: V769 Penunjuk 'buffer.get ()' dalam ekspresi 'line_buffer - buffer.get ()' sama dengan nullptr. Nilai yang dihasilkan tidak masuk akal dan tidak boleh digunakan. csv.hpp 4957
Sebenarnya kami berdua salah dan tidak ada kesalahan. Kita akan berbicara tentang nuansanya lebih lanjut, tetapi untuk saat ini mari kita mulai dengan yang sederhana.
Jadi, mari kita lihat mengapa berbahaya menulis kode seperti ini:
Foo(std::move(buffer), line_buffer - buffer.get());
Saya pikir Anda bisa menebak jawabannya. Hasilnya bergantung pada urutan argumen dievaluasi. Pertimbangkan ini di kode sintetis berikut:
#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;
}
Mari gunakan Compiler Explorer lagi dan lihat output dari program ini yang dikompilasi oleh kompiler yang berbeda.
Kompiler Clang 11.0.0. Hasil :
23387846
22
Penyusun GCC 10.2. Hasil :
22
26640070
Kami mengharapkan hasilnya, dan Anda tidak bisa menulis seperti itu. Inilah yang diperingatkan oleh penganalisis PVS-Studio.
Saya ingin mengakhiri ini, tetapi semuanya menjadi sedikit lebih rumit. Faktanya adalah bahwa kita berbicara tentang meneruskan argumen berdasarkan nilai, dan ketika membuat instance template fungsi std :: make_pair, semuanya akan berbeda. Mari terus selami perbedaannya dan cari tahu mengapa PVS-Studio salah dalam kasus ini.
std :: make_pair
Mari kita lihat situs web cppreference dan lihat bagaimana template fungsi std :: make_pair berubah .
Hingga C ++ 11:
template <kelas T1, kelas T2>Sejak C ++ 11, hingga C ++ 14:
std :: pasangan <T1, T2> make_pair (T1 t, T2 u);
template <kelas T1, kelas T2>Sejak C ++ 14:
std :: pair <V1, V2> make_pair (T1 && t, T2 && u);
template <kelas T1, kelas T2>Seperti yang Anda lihat, pada suatu waktu, std :: make_pair mengambil argumen berdasarkan nilai. Jika std :: unique_ptr ada saat itu , maka kode di atas memang salah. Apakah kode ini akan berhasil atau tidak adalah masalah keberuntungan. Dalam praktiknya, tentu saja, situasi ini tidak akan pernah muncul, karena std :: unique_ptr muncul di C ++ 11 sebagai pengganti std :: auto_ptr .
constexpr std :: pasangan <V1, V2> make_pair (T1 && t, T2 && u);
Mari kembali ke waktu kita. Dimulai dengan versi standar C ++ 11, konstruktor mulai menggunakan semantik bergerak.
Ada poin halus di sini bahwa std :: moveitu tidak benar-benar memindahkan apa pun, itu hanya mengubah objek menjadi referensi rvalue . Ini akan memungkinkan std :: make_pair untuk meneruskan pointer ke std :: unique_ptr baru , meninggalkan nullptr di smart pointer asli. Tetapi pengoperan pointer ini tidak akan terjadi sampai kita masuk ke dalam std :: make_pair . Pada saat itu, kami telah menghitung line_buffer - buffer.get () , dan semuanya akan baik-baik saja. Artinya, panggilan ke buffer.get () tidak bisa mengembalikan nullptrpada saat itu dihitung, terlepas dari kapan tepatnya itu terjadi.
Saya minta maaf untuk deskripsi yang rumit. Intinya adalah bahwa kode ini benar-benar valid. Dan faktanya, penganalisis statis PVS-Studio memberikan peringatan palsu dalam kasus ini. Namun, tim kami tidak yakin bahwa kami harus buru-buru membuat perubahan pada logika penganalisis untuk situasi seperti itu.
Raja sudah mati, panjang umur raja!
Kami menemukan bahwa operasi yang dijelaskan dalam artikel tersebut ternyata salah. Terima kasih kepada salah satu pembaca kami yang telah menarik perhatian kami pada kekhasan implementasi std :: make_pair .
Namun, ini adalah kasus ketika kami tidak yakin apakah layak untuk meningkatkan perilaku penganalisis. Intinya adalah kode ini terlalu berbelit-belit. Anda harus mengakui bahwa kode apa yang telah kami analisis tidak layak untuk penyelidikan rinci seperti itu, yang menyeret seluruh artikel. Jika kode ini membutuhkan banyak perhatian, maka itu adalah kode yang sangat buruk.
Sangat tepat di sini untuk mengingat artikel " Positif palsu adalah musuh kita, tetapi mungkin masih menjadi teman Anda ". Publikasi ini bukan milik kami, tapi kami setuju dengannya.
Ini mungkin masalahnya. Peringatan itu mungkin salah, tetapi ini menunjuk ke tempat yang lebih baik untuk melakukan refaktorisasi. Cukup menulis sesuatu seperti ini:
auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.push_back(
std::make_pair(std::move(buffer), delta));
Atau Anda dapat membuat kode lebih baik dalam situasi ini dengan menggunakan metode emplace_back :
auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.emplace_back(std::move(buffer), delta);
Kode ini akan membuat objek std :: pair yang dihasilkan di container "in place", melewati pembuatan objek sementara dan memindahkannya ke container. Omong-omong, penganalisis PVS-Studio menyarankan untuk melakukan penggantian seperti itu dengan mengeluarkan peringatan V823 dari kumpulan aturan untuk pengoptimalan mikro kode.
Kode ini akan menjadi lebih sederhana dan jelas bagi pembaca dan penganalisis manapun. Tidak ada gunanya menjejalkan sebanyak mungkin tindakan ke dalam satu baris kode.
Ya, dalam hal ini beruntung dan tidak ada kesalahan. Tetapi kecil kemungkinannya bahwa penulis, ketika menulis kode ini, mengingat semua yang kita diskusikan di kepalanya. Kemungkinan besar, itu adalah keberuntungan yang dimainkan. Dan di lain waktu Anda mungkin tidak beruntung.
Kesimpulan
Jadi, kami menemukan bahwa tidak ada kesalahan nyata. Penganalisis menghasilkan positif palsu. Mungkin kami akan menghapus peringatan hanya untuk kasus seperti itu, tetapi mungkin tidak. Kami akan memikirkannya nanti. Bagaimanapun, ini adalah kasus yang agak jarang, dan kode di mana argumen dievaluasi dengan efek samping berbahaya secara umum, dan lebih baik menghindarinya. Perlu dilakukan refactoring setidaknya untuk tujuan pencegahan.
Lihat kode:
Foo(std::move(buffer), line_buffer - buffer.get());
mudah dipatahkan dengan mengubah sesuatu di tempat lain dalam program. Kode seperti ini sulit untuk dipertahankan. Dan itu juga tidak menyenangkan karena dapat memberikan perasaan yang salah bahwa semuanya bekerja dengan benar. Faktanya, ini hanya kebetulan, dan semuanya bisa rusak jika Anda mengubah compiler atau pengaturan optimasi.
Jadikan kode Anda lebih mudah!
Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Andrey Karpov. Penganalisis Kode salah. Hidup Analyzer! ...