Delete warnings

[200km]

Overview

Delete warnings

Details

There are so many warnings in S2E build. We need to delete them.

Conditions for close

• all warnings are deleted.

Supplement

NA

Note

NA

[200km]

#92 と #93　でだいぶ減った。残りもう少し頑張る。

[200km]

@sksat 外部からコピーしてきているコードのwarningも気にせず修正していっていいですかね？

[200km]

#97 ここまでのPRがすべて通れば、g++のwarningsはなくなるはず。
Windows Visual Studio側も確認が必要。

[sksat]

@sksat 外部からコピーしてきているコードのwarningも気にせず修正していっていいですかね？

いいと思います

[200km]

@sksat 外部からコピーしてるコードに #pragmra warningを使っているものがありました。このあたりどうしましょうか。

[200km]

VS2019でビルドしたら、111件warningがでた。。。
VSとg++で警告オプションのレベルを似たものにしたいですね。

[200km]

現状整理

• g++
  • 警告オプションは -Wall
  • #pragma warningしているよwarning以外は消えた
• VS2019
  • 警告オプションは /W4
  • 111件のwarning

[200km]

@sksat ワーニングの件、アドバイスいただけると嬉しいです。
方針として、

1. VS側の警告レベルを下げてg++に合わせる
2. g++側の警告レベルを上げてVSに合わせる
3. いまの警告レベルのままでとにかくVS側もwarningを削る

の三択で、2が一番良いのだろうなと思いつつ、警告レベルの設定をよく理解しておらずどうしようかなと言う感じです。

[sksat]

とりあえずg++に-Wextraを追加かな，と思います(GCCは-Wall -Wextraしてようやくほぼすべてのワーニングが出るので)．

[200km]

Thanks. I agree with you.

[200km]

After we added the -Wextra, we can see 1300 warnings in g++...
I will tackle to delete them all.

[200km]

@sksat There are so many unused parameter warnings for function's arguments.
Can I use UNUSED macro like here?
Or do you have an another recommendation?

[sksat]

UNUSEDマクロはせっかく出ているワーニングを握り潰すということなので，あんまりやりたくはないですね...
これは他のuserとかから使われていて変更のためのコストが大きい，そもそも後から引数を使うように実装が変わる可能性がある，ということですかね？

[200km]

これは他のuserとかから使われていて変更のためのコストが大きい

Yes, changing the function's arguments makes huge effect to all users. But if it is really need to change, I can change it.

そもそも後から引数を使うように実装が変わる可能性がある

There are several examples we cannot remove the arguments

• We use the arguments in HILS mode.
  • https://github.com/ut-issl/s2e-core/blob/develop/src/Interface/HilsInOut/HilsPortManager.cpp#L9
• Abstract class has the arguments, but some inheritance classes don't use the arguments
  • https://github.com/ut-issl/s2e-core/blob/develop/src/Component/AOCS/Gyro.cpp#L15

[sksat]

なるほどです．じゃあ仕方ないですかね．
ワーニングを握り潰すとはいってもUNUSEDで握り潰している箇所を明示はできるわけですし，一旦UNUSEDでやりましょう．

[200km]

Thanks. I will make another issue to remove UNUSED.

[200km]

All warnings are deleted again for g++ by #106.

[200km]

We still have 57 warnings in VS2022.

@sksat gcc側にもう少し警告オプション追加する必要がありそうですが，どれを入れるべきかというのが分かっておらず，教えてもらえると嬉しいです．

[200km]

とりあえずVS2022を使ってこれらのwarningsを消していきます．

[200km]

@seki-hiro I2cObcTarget周りでHILS用に追加実装してくれた部分で，uint/int関連のワーニングが出ているので修正してもらえますか？いろんな関数が関連していて，実装者本人にやってもらった方が正確そうなので．

[200km]

この記事のオプション設定を参考にしたりすると良いのでしょうか？

[200km]

g++について上の記事のwarning設定にしてみたところ、次のような感じになった。

何もwarningが出なかったもの

-Wcast-align -Wctor-dtor-privacy -Wdisabled-optimization -Winit-self -Wlogical-op -Wmissing-include-dirs -Wnoexcept -Wredundant-decls -Wsign-promo -Wstrict-null-sentinel -Wundef -Wno-unused

warning出たもの

• -pedantic : +1k
• -Wcast-qual : +4
• -Wformat=2 : +1
• -Wmissing-declarations : +14
• -Wold-style-cast : +298
• -Woverloaded-virtual : +4
• -Wshadow : +26
• -Wsign-conversion : +2k
• -Wstrict-overflow=5 : +15
• -Wswitch-default : +1

ただ、Visual Studioと同じようなwarningが出ているようには見えなかった。表示が違うだけで一緒なのかもしれないけれど。

[sksat]

ワーニングというのはコンパイラがそれぞれかなり独自に出しているものなので，規格に絡むもの以外は他のコンパイラ実装でまったく同じものが出るということはないです(なんだかんだ似ていることはありますけどね)．とりあえず最優先で入れるべきなのは-Wpedanticかな(-pedanticと同じ意味です)．

[sksat]

-Wsign-conversionもまあ必要だな(というかそんなに出るのか)

[200km]

完全に一致するとは思っていないのですが、似たwarningsが出てくれれば、一方で消せば他方も消えるので嬉しいなと思っていました。最終的にはどちらも見て、一方で出てたら修正するしか無いという感じですかね。

とりあえず、VSで出ないようにします。

[200km]

-Wpedantic　と　-Wsign-conversion　はその後入れるのを検討しますね。
前者は変なところに;がついている、後者は配列の要素指定にintを使っているというのがほとんどだったので、修正できるかなとは思っています。

[sksat]

ですねー．両方見るしかないです．あーやけに多いなと思ったらそういうことか．なるほど．

[200km]

残っているwarnings

g++

• #pragma warning関連

VS2022

• algorithm/transformを使っているIniReader.cpp
• defaultlib LIBCMT conflicts with use of other libs; use /NODEFAULTLIB:library

[200km]

IniReaderの方はこの記事を参考に修正

LIBCMTはこの記事を参考に[/MT]をつけて修正。ExtLibも同じような設定にする必要あり。

[200km]

残りはg++の#pragma warning関連のみ。

[200km]

#pragma warning関連 も片付けた。これで、一旦VS2022でもgccでもwarningは撲滅できた

[200km]

-Wpedanticについては、入れ込めてwarningも全部消した。動作に影響が出るような修正ではなかった。
-Wsign-conversionは入れ込むの大変そう。

[200km]

-Wsign-conversionは入れ込むの大変そう。

入れようと思うと結構大きな改修になるので、v5.0.0に入れれそうにない。
下記issueで別対応とする。
#120