You make very valid points. I will consider using the -isystem flag. However i 
would like to point out that i am not expecting protobuf itself to compile with 
all the same warnings strictures that my code does - indeed that would be 
unreasonable. But i think providing warning-free headers is rather more 
reasonable to expect, and indeed comparable to the generated code situation 
that you mention, since i must obviously compile the headers with my project's 
flags. Treating them as system headers should not really be needed, as after 
all they are not in fact. Thoughts?

-original message-
Subject: Re: [protobuf] Re: 2.3.0 released
From: Kenton Varda <ken...@google.com>
Date: 11.01.2010 20.35

We get a lot of complaints about warnings in our code.  When the warnings
occur in generated code, I generally fix them (because generated code
generally must be compiled using your project's flags), but for warnings in
protobuf code my answer is that you need to include the protobuf headers as
system headers so that warnings are ignored.  Different projects make
different choices about what warnings to enable on their code, so it seems
unreasonable to expect that every one of your dependencies enable at least
all of the warnings that you enable.  The sign comparison warnings are
particularly annoying because they warn about every line that looks like:
  for (int i = 0; i < some_vector.size(); i++)
which are very common and perfectly valid.  Therefore we compile with
-fno-sign-compare.

That said, this patch seems harmless, so I'll submit it.  But I won't be
doing a new release just for this, and new warnings could easily be
introduced before the next release.

On Sun, Jan 10, 2010 at 3:13 AM, edan <eda...@gmail.com> wrote:

> I looks like a good workaround - thanks for the info.
> I will wait and see if Kenton is planning to fix this, then decide my next
> steps.
> --edan
>
>
> On Sun, Jan 10, 2010 at 10:59 AM, Monty Taylor <mord...@inaugust.com>wrote:
>
>>
>>
>> edan wrote:
>> > I happily upgraded to 2.3.0 - I always like to take the latest and
>> greatest.
>> > Unfortunately, and I think for the first time ever while upgrading
>> > protobuf, I ran into a problem!
>> > We compile our code with "-Werror", and this bombed out on a header
>> file:
>>
>> We build with errors on in our project too - our solution to this has
>> been to:
>>
>> a) install the headers into a system locatiom, at which point gcc will
>> not issue warnings for them. It looks like you did this in the context
>> of your /devfs location - perhaps you need to change some system configs
>> to completely understand that location as a chroot?
>>
>> b) If they aren't in a system location, include them via -isystem rather
>> than -I, which will have the same effect.
>>
>> > cc1plus: warnings being treated as errors
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h: In member
>> > function "bool
>> >
>> google::protobuf::io::CodedInputStream::ReadLittleEndian32(google::protobuf::uint32*)":
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h:776:
>> > warning: comparison between signed and unsigned integer expressions
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h: In member
>> > function "bool
>> >
>> google::protobuf::io::CodedInputStream::ReadLittleEndian64(google::protobuf::uint64*)":
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h:791:
>> > warning: comparison between signed and unsigned integer expressions
>> >
>> >
>> > My patch to fix this was:
>> >
>> > ====
>> >
>> //depot/project/zenith/ports/protobuf/std/build/src/google/protobuf/io/coded_stream.h#2
>> > (ktext) -
>> >
>> //depot/project/zenith/ports/protobuf/std/build/src/google/protobuf/io/coded_stream.h#3
>> > (ktext) ==== content
>> > 776c776
>> > <   if (GOOGLE_PREDICT_TRUE(BufferSize() >= sizeof(*value))) {
>> > ---
>> >>   if (GOOGLE_PREDICT_TRUE(BufferSize() >=
>> > static_cast<int>(sizeof(*value)))) {
>> > 791c791
>> > <   if (GOOGLE_PREDICT_TRUE(BufferSize() >= sizeof(*value))) {
>> > ---
>> >>   if (GOOGLE_PREDICT_TRUE(BufferSize() >=
>> > static_cast<int>(sizeof(*value)))) {
>> >
>> > Any chance you can patch this and re-release?  I'd really like to have
>> > un-patched code in our product, but I can't use 2.3.0 without this
>> patch.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to proto...@googlegroups.com.
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en.


Reply via email to