#1851 closed defect (fixed)
[PATCH] Build warnings
Reported by: | zoot | Owned by: | ben |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 14 |
Component: | Build & Packages | Keywords: | patch |
Cc: | Patch: |
Description
I get the warnings attached below when building the game. It compiles fine, but it would easier to parse the error output if they were fixed.
Attachments (4)
Change History (23)
by , 11 years ago
Attachment: | err.make.txt added |
---|
comment:1 by , 11 years ago
by , 11 years ago
Attachment: | clean-up-build-with-gcc-4.8.1.patch added |
---|
follow-up: 10 comment:4 by , 11 years ago
Keywords: | patch review added |
---|---|
Milestone: | Backlog → Alpha 14 |
Summary: | Build warnings → [PATCH] Build warnings |
One warning is still unfixed:
In PatchRData.cpp:236 the m_TileMask is a u16. It gets some bits set and then its assigned to a u8, causing a narrowing warning.
The u16 has bits set that will be lost in a u8. So I dont know if this is actually okay or if we need to expand the u8 to 16 bits.
comment:6 by , 11 years ago
Keywords: | review removed |
---|
Thanks for the patch. I asked Philip about PatchRData, let's see what/when he answers.
comment:7 by , 11 years ago
About m_TileMask
, there's a comment saying
// bit n set if this blend contains neighbour tile BlendOffsets[n]
which if I'm reading it correctly means it only uses the lower 8 bits (in a tile map, each tile can only be bordered by 8 other tiles). So I think it was a mistake and should be u8
.
comment:8 by , 11 years ago
Hmm, no I was wrong, it does use the 9th bit for something. But I don't think it's necessary to copy that bit to the u8
, so maybe a typecast is enough.
follow-up: 11 comment:10 by , 11 years ago
Replying to Markus:
One warning is still unfixed:
I am still getting some warnings in fcollada, but I suppose this is expected. I am also getting this 0 A.D. warning:
In file included from ../../../source/simulation2/tests/test_Serializer.cpp:15:0: ../../../source/simulation2/tests/../../../source/simulation2/tests/test_Serializer.h: In member function ‘void TestSerializerPerf::test_hash_DISABLED()’: ../../../source/simulation2/tests/../../../source/simulation2/tests/test_Serializer.h:668:3: warning: variable ‘_qzz_res’ set but not used [-Wunused-but-set-variable] ../../../source/simulation2/tests/../../../source/simulation2/tests/test_Serializer.h:675:3: warning: variable ‘_qzz_res’ set but not used [-Wunused-but-set-variable]
comment:11 by , 11 years ago
Replying to fabio:
Replying to Markus:
One warning is still unfixed:
I am still getting some warnings in fcollada, but I suppose this is expected. I am also getting this 0 A.D. warning
Would be fixed with r1971.
comment:12 by , 11 years ago
OK. I also reported mongoose warning here: https://github.com/valenok/mongoose/issues/183
There is already a reply.
by , 11 years ago
Attachment: | clean-up-fcollada-build-with-gcc-4.8.1.patch added |
---|
follow-up: 14 comment:13 by , 11 years ago
Keywords: | review added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
comment:14 by , 11 years ago
Replying to Markus:
Can you post the warnings relevant to the FCollada patch?
by , 11 years ago
Attachment: | fcollada.warnings added |
---|
comment:16 by , 11 years ago
Keywords: | review removed |
---|
I fixed most of the warnings:
- for the printf-style argument, it's simpler to cast the argument as
unsigned long
so it still works with%lu
- it was easier to add
Wno-unused-but-set-variable
to theMakefile
, as it was unclear if the return value assignments were needed to avoid optimizations (note almost all of them occur in functions that force templated functions to be exported) or warnings in some other compiler/analysis tool -- or, since it's not broken, why fix it? :)
I haven't tested if r13629 compiles or fixes the warnings, as I'm away from a Linux machine, so please test.
comment:17 by , 11 years ago
I just did a clean rebuild, everything looks good; no warnings or errors.
comment:19 by , 11 years ago
r13798 reverted the change to the Makefile as this breaks the build on at least llvm-gcc-4.2 on OS X (and on GCC 4.2).
Which compiler is this?