New issue 361 by NN1436...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

VC2008 with level warning 4 produces several warnings:

1>google\protobuf\stubs\common.h(1201) : warning  
C4512: 'google::protobuf::FatalException' : assignment operator could not  
be generated

1>google\protobuf\io\coded_stream.h(901) : warning C4244: '=' : conversion  
from 'google::protobuf::uint32' to google::protobuf::uint8', possible loss  
of data

1>google\protobuf\io\coded_stream.h(965) : warning C4244: '=' : conversion  
from 'google::protobuf::uint32' to google::protobuf::uint8', possible loss  
of data

Fix it please.

Comment #1 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

gcc also generates a lot of errors "comparison between signed and unsigned  
integer expressions" if protobuf is compiled with flags "-Wall -w9 -Werror"

Comment #2 on issue 361 by jacobgla...@yahoo.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

There's also a bunch of size mismatches that I'd love to see fixed.

Comment #3 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

There are also few warnings I don't know how to handle properly

protobuf-2.4.1\src\google\protobuf\text_format.cc:126: warning:  
C4355: 'this' : used in base member initializer list
protobuf-2.4.1\src\google\protobuf\compiler\importer.cc:118: warning:  
C4355: 'this' : used in base member initializer list

protobuf-2.4.1\src\google\protobuf\text_format.cc:423: warning:  
C4800: 'google::protobuf::uint64' : forcing value to bool 'true' or 'false'  
(performance warning)
protobuf-2.4.1\src\google\protobuf\text_format.cc:423: warning:  
C4800: 'google::protobuf::uint64' : forcing value to bool 'true' or 'false'  
(performance warning)
protobuf-2.4.1\src\google\protobuf\generated_message_reflection.cc:1149:  
warning: C4800: 'const google::protobuf::uint32' : forcing value to  
bool 'true' or 'false' (performance warning)

Comment #4 on issue 361 by NN1436...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

C4800: 'google::protobuf::uint64' : forcing value to bool 'true' or 'false'
This is simple write the correct expression.

Generates warning:
uint64 x = 1;
bool a = x;

No warning, and makes code more readable.
uint64 x = 1;
bool a = x != 0;

About this in base member initializer list, it can be problem and can be  
false alarm.
It depends.

Problematic code:
// Some interface
class I { virtual void Do() = 0; };

class A { public: A(I& i) { i.Do(); } };
class B : public A { public: B() : A(*this) { } } // BOOM !

Non problematic code, in case we just store reference/pointer.
// Some interface
class I { virtual void Do() = 0; };

class A { public: A(I& i) : _i(i) { } private: I& _i; };
class B : public A { public: B() : A(*this) { } } // No problems.

If it is the second case, disable warning by

#pragma warning(push)
#pragma warning(disable: 4355)

... Code

#pragma warning(pop)

Of course it is for MSVC, I don't know if GCC complains about it.

Comment #5 on issue 361 by liuj...@google.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

(No comment was entered for this change.)

Comment #6 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

Thanks.

What about this:
src\google\protobuf\stubs\common_unittest.cc:75: warning: C4307: '+' :  
integral constant overflow

EXPECT_EQ(0u, kuint32max + 1);

Seems like this is done intentionally. Should it also be supressed with  
pragma?

Comment #7 on issue 361 by tom.ritc...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

In my setup, at least, GCC doesn't complain about "forcing value to bool",  
or about "'this' : used in base member initializer list".

I frankly think the "forcing value to bool" is a bogus error that should be  
suppressed in everyone's project.  I honestly don't believe there is any  
significant "performance" penalty at all - the last time I looked at the  
generated code for bool a = x; and bool a = (x != 0); they were exactly the  
same.

The unit test warning probably should be suppressed with a pragma BUT if it  
isn't bothering people within Google IMHO this is a much lower priority.

As an external developer who makes heavy use of protocol buffers, I only  
ever ran the Google unit tests one time, and I really didn't care if there  
were a few warnings there.  But warnings in my main build are troublesome  
because I like to have none at all so I can clearly see "real" warnings  
when they come up.

Comment #8 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

I've fixed all warnings in protobuf itself, only warnings for unittests  
left:

google/protobuf/compiler/cpp/cpp_unittest.cc:878:5: ошибка: case value  
«12589235» not in enumerated type «protobuf_unittest::TestSparseEnum»  
[-Werror=switch]

Comment #9 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

This patch is for protobuf version 2.4.1

There several types of warnings left: unused-variable, conversion-null,  
overflow, switch

All signed versus unsinged warnings fixed (using explicit static_cast).

Attachments:
        fix_warnings_2.4.1.patch  151 KB

Comment #10 on issue 361 by zabulu...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

This patch fixes issues topic starter posted about

Attachments:
        1.patch  1.8 KB

Comment #11 on issue 361 by xiaof...@google.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

"int" is used as the loop counter intentionally. We would rather disable  
the warning instead of using "size_t".

Comment #12 on issue 361 by Van...@gmail.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

I don't see the reason why? If size() returns size_t, why then i should be  
int it the following statement:

for (i = 0; i < input_files_.size(); i++)

Comment #13 on issue 361 by xiaof...@google.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

It's the convention at Google to use "int" as loop counters, which makes it  
pointless to update the code to use "size_t" as it's inevitable that new  
code will introduce more "int"s in the future.
Unless Google's style guide discourages the use of "int", we shouldn't  
bother to make the change.

Comment #14 on issue 361 by jacobgla...@yahoo.com: Fix warnings
http://code.google.com/p/protobuf/issues/detail?id=361

How about something like:

const int n = static_cast<int>(input_files_.size());
for (i = 0; i < n; i++)