Tools: waf: add various conversion warning compiler options #11094
Conversation
peterbarker
force-pushed
the
pr/more-compiler-warnins
branch
3 times, most recently
from
aa4ab6a
to
f70f44f
Compare
peterbarker
force-pushed
the
pr/more-compiler-warnins
branch
2 times, most recently
from
c8e8c6f
to
e10cde9
Compare
Tools/ardupilotwaf/boards.py
Outdated
| '-Werror=int-conversion', | ||
|
|
||
| # catch conversion issues: | ||
| "-Werror=string-conversion", |
There was a problem hiding this comment.
Use single quote please (from here down).
Why are these only for Clang? They don't exist in GCC?
There was a problem hiding this comment.
My gcc only seems to have float-conversion and int-conversion. Whatever we run on CI didn't even seem to have those.
Tools/ardupilotwaf/boards.py
Outdated
| '-Werror=float-equal', | ||
|
|
||
| # catch conversion errors: | ||
| # '-Werror=float-conversion', # we do a lot of this... |
There was a problem hiding this comment.
I've narrowed and narrowed and narrowed these flags as I discovered our compilers in CI didn't support these flags.
There was a problem hiding this comment.
In CI we are running the same version for SITL and ChibiOS - 4.9. I would support moving all forward (with Linux too), but the decision isn't mine.
.travis.yml
Outdated
| @@ -71,7 +71,7 @@ matrix: | |||
| env: CI_BUILD_TARGET="sitltest-quadplane sitltest-plane" | |||
| - if: type != cron | |||
| compiler: "clang-7" | |||
| env: CI_BUILD_TARGET="sitltest-rover sitltest-sub"" | |||
| env: CI_BUILD_TARGET="sitl sitltest-rover sitltest-sub"" | |||
There was a problem hiding this comment.
No, no, no, this already takes way too long and we can't take SITL out of Semaphore without breaking release branches. If it is desired to build more boards with Clang then we should look at supporting it on Semaphore.
There was a problem hiding this comment.
This is what was decided on the devcall. It was thought that the incremental time to build all SITL targets here would be minimal.
@WickedShell measured this at an additional 8 minutes (23 -> 31 minutes).
My testing here seems to reflect that 30% premium when not using ccache:
ccache in play:
with SITL:
real 10m3.126s
user 10m44.149s
sys 2m41.884s
without SITL:
real 9m47.852s
user 10m15.686s
sys 2m36.781s
with SITL (again):
real 10m9.875s
user 10m25.704s
sys 2m33.524s
ccache --clear and rm -rf build
with SITL:
real 19m28.768s
user 10m12.861s
sys 2m25.805s
without SITL:
real 14m34.731s
user 9m43.164s
sys 2m23.088s
Why is ccache false in .travis.yml? Weren't we actually preserving our ccache at one stage with CI?
There was a problem hiding this comment.
We have ccache in CI, but saving it between runs isn't beneficial, after some time it's actually worse to have it - so it has been removed.
It's also not very useful for sitl vs sitltest because sitltest might be running a different version of pymavlink - and that alone might make a large majority of files get recompiled.
In Travis it was 6m20s more compared to latest master. Unfortunately sitltest builds aren't showing the ccache after the build so we don't know hit values, but looking at build times, Rover didn't benefit much (4m20s vs 3m53s), Sub was much faster (4m2s vs 8s).
This is a lot of time and our jobs are already very unbalanced - making them more unbalanced is a bad idea.
peterbarker
force-pushed
the
pr/more-compiler-warnins
branch
2 times, most recently
from
d9f2105
to
9f6a641
Compare
|
@OXINARF please check if your issues are resolved here |
|
The outcome from the dev call was that this looked OK to merge but it seemed like @OXINARF's requests hadn't been completed yet. PeterB will remove some of the commented out lines and then @OXINARF would respond whether he's happy or not to merge. @OXINARF, once you're happy please feel free to merge to master. |
peterbarker
force-pushed
the
pr/more-compiler-warnins
branch
from
9f6a641
to
23745ec
Compare
This makes the arbitrary decision that arming checks always report
failures to the GCS.
Fixes:
In file included from ../../ArduCopter/events.cpp:1:
In file included from ../../ArduCopter/Copter.h:91:
../../ArduCopter/AP_Arming.h:33:69: fatal error: non-virtual member function marked 'override' hides virtual member function
bool arm_checks(bool display_failure, AP_Arming::Method method) override;
^
../../libraries/AP_Arming/AP_Arming.h:64:18: note: hidden overloaded virtual function 'AP_Arming::arm_checks' declared here: different number of parameters (1 vs 2)
virtual bool arm_checks(AP_Arming::Method method);
^
1 error generated.
peterbarker
force-pushed
the
pr/more-compiler-warnins
branch
from
23745ec
to
4b63142
Compare
|
Please note the additional patch here required to get things to compile. master is currently broken clang++ users. |
suggested by @patrickelectric