Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#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)

err.make.txt (4.8 KB ) - added by zoot 11 years ago.
clean-up-build-with-gcc-4.8.1.patch (7.8 KB ) - added by Markus 11 years ago.
clean-up-fcollada-build-with-gcc-4.8.1.patch (6.9 KB ) - added by Markus 11 years ago.
fcollada.warnings (6.7 KB ) - added by Markus 11 years ago.

Download all attachments as: .zip

Change History (23)

by zoot, 11 years ago

Attachment: err.make.txt added

comment:1 by historic_bruno, 11 years ago

Which compiler is this?

comment:2 by leper, 11 years ago

I'd guess gcc 4.7 or higher.

comment:3 by zoot, 11 years ago

Yes, gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2.

comment:4 by Markus, 11 years ago

Keywords: patch review added
Milestone: BacklogAlpha 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.

Last edited 11 years ago by Markus (previous) (diff)

comment:5 by leper, 11 years ago

In 13469:

Fix build warnings. Patch by Markus. Refs #1851.

comment:6 by leper, 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 historic_bruno, 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.

Last edited 11 years ago by historic_bruno (previous) (diff)

comment:8 by historic_bruno, 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.

comment:9 by ben, 11 years ago

Owner: set to ben
Resolution: fixed
Status: newclosed

In 13470:

Hopefully fixes GCC warning about narrowing conversion in C++11, fixes #1851

in reply to:  4 ; comment:10 by fabio, 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]

in reply to:  10 comment:11 by Markus, 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

The latter would be fixed with #1971.

fcollada is another topic :-/

Last edited 11 years ago by Markus (previous) (diff)

comment:12 by fabio, 11 years ago

OK. I also reported mongoose warning here: https://github.com/valenok/mongoose/issues/183

There is already a reply.

comment:13 by Markus, 11 years ago

Keywords: review added
Resolution: fixed
Status: closedreopened

in reply to:  13 comment:14 by historic_bruno, 11 years ago

Replying to Markus:

Can you post the warnings relevant to the FCollada patch?

by Markus, 11 years ago

Attachment: fcollada.warnings added

comment:15 by ben, 11 years ago

Resolution: fixed
Status: reopenedclosed

In 13629:

Fixes FCollada build warnings with GCC, based on patch by Markus, fixes #1851

comment:16 by historic_bruno, 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 the Makefile, 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 zoot, 11 years ago

I just did a clean rebuild, everything looks good; no warnings or errors.

comment:18 by historic_bruno, 11 years ago

Great :)

comment:19 by leper, 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).

Note: See TracTickets for help on using tickets.