署名された/署名されていない警告の大部分に対する許容可能な修正
-
07-07-2019 - |
質問
私自身は、中に含まれる値が負になることはありませんが、ほとんどの場合、符号付き整数に取り組んでいるプロジェクトで最良の選択であると確信しています。 (ループの単純なリバース、バグの可能性の減少など、特に0〜20の値しか保持できない整数の場合など)
これがうまくいかない場所の大部分は、std :: vectorの単純な繰り返しです。これは、以前は配列であったことが多く、後でstd :: vectorに変更されました。したがって、これらのループは一般的に次のようになります。
for (int i = 0; i < someVector.size(); ++i) { /* do stuff */ }
このパターンは頻繁に使用されるため、符号付き型と符号なし型のこの比較に関するコンパイラ警告スパムの量は、より有用な警告を隠す傾向があります。 INT_MAX以上の要素を持つベクターは絶対に存在しないことに注意してください。これまで、コンパイラの警告を修正するために2つの方法を使用していたことに注意してください。
for (unsigned i = 0; i < someVector.size(); ++i) { /*do stuff*/ }
これは通常は機能しますが、ループに「if(i-1 <!> gt; = 0)...」などのコードが含まれている場合、静かに中断する可能性があります。
for (int i = 0; i < static_cast<int>(someVector.size()); ++i) { /*do stuff*/ }
この変更には副作用はありませんが、ループの可読性が大幅に低下します。 (そして、さらに入力します。)
だから私は次のアイデアを思いつきました:
template <typename T> struct vector : public std::vector<T>
{
typedef std::vector<T> base;
int size() const { return base::size(); }
int max_size() const { return base::max_size(); }
int capacity() const { return base::capacity(); }
vector() : base() {}
vector(int n) : base(n) {}
vector(int n, const T& t) : base(n, t) {}
vector(const base& other) : base(other) {}
};
template <typename Key, typename Data> struct map : public std::map<Key, Data>
{
typedef std::map<Key, Data> base;
typedef typename base::key_compare key_compare;
int size() const { return base::size(); }
int max_size() const { return base::max_size(); }
int erase(const Key& k) { return base::erase(k); }
int count(const Key& k) { return base::count(k); }
map() : base() {}
map(const key_compare& comp) : base(comp) {}
template <class InputIterator> map(InputIterator f, InputIterator l) : base(f, l) {}
template <class InputIterator> map(InputIterator f, InputIterator l, const key_compare& comp) : base(f, l, comp) {}
map(const base& other) : base(other) {}
};
// TODO: similar code for other container types
基本的に、size_typeを返すメソッドがオーバーライドされて「int」のみを返すSTLクラスが表示されます。これらは継承されないため、コンストラクタが必要です。
既存のコードベースにこのようなソリューションが見られる場合、これを開発者としてどう思いますか?
「なんて、彼らはSTLを再定義している、なんて巨大なWTF!」と思うか、これはバグを防ぎ読みやすさを向上させる素敵なシンプルなソリューションだと思いますか?または、これらのすべてのループを変更してstd :: vector <!> lt; <!> gt; :: iterator?
を使用するのに1日(半)ほど費やしたことを見た方がいいかもしれません(特に、このソリューションが生データ(unsigned charなど)およびビットマスク以外の符号なし型の使用を禁止することと組み合わされた場合)
解決
このコミュニティwikiを作成しました...編集してください。 <!> quot; int <!> quotに対するアドバイスに同意しません。もう。今では悪くないと思う。
はい、リチャードに同意します。これらのようなループでは、カウント変数として'int'
を使用しないでください。インデックスを使用してさまざまなループを実行する方法を次に示します(理由はほとんどありませんが、これが役立つ場合もあります)。
転送
for(std::vector<int>::size_type i = 0; i < someVector.size(); i++) {
/* ... */
}
後方
これを行うことができます。これは完全に定義された動作です:
for(std::vector<int>::size_type i = someVector.size() - 1;
i != (std::vector<int>::size_type) -1; i--) {
/* ... */
}
まもなく、c ++ 1x(次のC ++バージョン)がうまく機能すると、次のようにできます:
for(auto i = someVector.size() - 1; i != (decltype(i)) -1; i--) {
/* ... */
}
0未満にデクリメントすると、iは符号なしのためラップアラウンドします。
ただし、署名されていない場合、バグが丸見えになります
それは、間違った方法(23.1 p5 Container Requirements
を使用)にするための引数であってはなりません。
上記のstd :: size_tを使用しない理由
C ++標準では、T::size_type
でT
が定義され、Container
はstd::size_t
であり、この型は符号なし整数型で定義された実装です。さて、上記のi
に(std::size_t)-1
を使用すると、バグが静かに丸lurみになります。 someVector.size() == 0
が<=>より小さいか大きい場合、<=>がオーバーフローするか、<=>の場合は<=>に到達しません。同様に、ループの状態は完全に壊れていたはずです。
他のヒント
STLコンテナから一般に派生しないでください。それらは、誰かがベースへのポインタを介してオブジェクトの1つを削除した場合、未定義の動作を呼び出す非仮想デストラクタを備えています。導出する必要がある場合ベクターからプライベートに実行し、using
宣言で公開する必要がある部分を公開します。
ここでは、ループ変数としてsize_t
を使用します。シンプルで読みやすいです。 int
インデックスを使用するとn00bが正しいと表示されるとコメントした投稿者。ただし、イテレータを使用してベクトルをループすると、経験豊富なn00bとして表示されます。これは、ベクトルの添字演算子が一定時間であることを認識していない人です。 (vector<T>::size_type
は正確ですが、不必要に冗長なIMO)。
<!> quot;イテレータを使用するとは思わないが、そうでない場合はn00b <!> quot;は、std :: vectorから派生した問題の良い解決策です。
最初に、開発者はvectorがstd:.vectorであり、mapがstd :: mapであることを期待しています。第二に、ソリューションは他のコンテナや、コンテナとやり取りする他のクラス/ライブラリに合わせて拡張できません。
はい、イテレータは見苦しく、イテレータループは読みづらく、typedefは混乱を隠しているだけです。しかし、少なくとも、それらはスケーリングし、標準的なソリューションです。
私の解決策は? stl-for-eachマクロ。それは問題がないわけではありません(主にマクロです、ヤック)が、意味を理解しています。それは、例えばこれですが、仕事はします。
イテレータを確実に使用します。すぐに、「auto」タイプを使用できるようになります。読みやすいように(懸念事項の1つ)、次のようにします。
for (auto i = someVector.begin();
i != someVector.end();
++i)
インデックスをスキップ
最も簡単なアプローチは、イテレーター、範囲ベースのforループ、またはアルゴリズムを使用して問題を回避することです:
for (auto it = begin(v); it != end(v); ++it) { ... }
for (const auto &x : v) { ... }
std::for_each(v.begin(), v.end(), ...);
これは、実際にインデックス値が必要ない場合に適したソリューションです。また、逆ループも簡単に処理できます。
適切な符号なしタイプを使用
別のアプローチは、コンテナのサイズタイプを使用することです。
for (std::vector<T>::size_type i = 0; i < v.size(); ++i) { ... }
std::size_t
も使用できます(<!> lt; cstddef <!> gt;から)。 std::vector<T>::size_type
はsize_type
と同じ型ではないかもしれないと(正しく)指摘する人がいます(通常はそうですが)。ただし、コンテナのint
がsize_as_int
に収まることを保証できます。したがって、逆ループに特定のスタイルを使用しない限り、すべてが問題ありません。逆ループの私の好みのスタイルはこれです:
for (std::size_t i = v.size(); i-- > 0; ) { ... }
このスタイルを使用すると、<=>より大きい型であっても<=>を安全に使用できます。他のいくつかの回答に示されている逆ループのスタイルでは、-1を正確に正しい型にキャストする必要があるため、入力しやすい<=>を使用できません。
署名されたタイプを使用します(慎重に!)
本当に署名付きタイプを使用する場合(またはスタイルガイドは実際に必要です)、たとえば<=>の場合、この小さな関数テンプレートを使用して、デバッグビルドの基礎となる仮定をチェックし、変換を明示的にしてコンパイラの警告を受け取らないようにしますメッセージ:
#include <cassert>
#include <cstddef>
#include <limits>
template <typename ContainerType>
constexpr int size_as_int(const ContainerType &c) {
const auto size = c.size(); // if no auto, use `typename ContainerType::size_type`
assert(size <= static_cast<std::size_t>(std::numeric_limits<int>::max()));
return static_cast<int>(size);
}
次のように書くことができます:
for (int i = 0; i < size_as_int(v); ++i) { ... }
または従来の方法でループを逆にします:
for (int i = size_as_int(v) - 1; i >= 0; --i) { ... }
<=>トリックは、暗黙的な変換を使用したループよりもタイピングが少しだけ多く、実行時に基本的な仮定を確認し、明示的なキャストでコンパイラの警告を黙らせ、非デバッグビルドと同じ速度を取得しますほぼ確実にインライン化され、最適化されたオブジェクトコードは、コンパイラがまだ暗黙的に実行していないことをテンプレートが実行しないため、これ以上大きくはなりません。
あなたは問題を再考しています。
size_t変数を使用することをお勧めしますが、プログラマが符号なしを正しく使用することを信用していない場合は、キャストを行い、andさを処理してください。それらをすべて変更するインターンを取得し、その後は心配しないでください。エラーとして警告をオンにすると、新しい警告が忍び寄ることはありません。ループは<!> quot; ugly <!> quot;今、しかし、あなたはあなたの宗教的姿勢の結果として署名されたものと署名されていないものを理解することができます。
vector.size()
はsize_t
varを返すため、int
を<=>に変更するだけで問題ありません。
Richardの答えはより単純ですが、単純なループでは多くの作業が必要になる点が異なります。