[cfe-commits] [PATCH] [libcxx] cleanup a few compile warnings emitted by GCC

Howard Hinnant hhinnant at apple.com
Thu Dec 27 17:09:31 PST 2012


On Dec 27, 2012, at 7:36 PM, Chandler Carruth <chandlerc at google.com> wrote:

>> On Thu, Dec 27, 2012 at 4:22 PM, Joerg Sonnenberger <joerg at britannica.bec.de> wrote:
>> On Thu, Dec 27, 2012 at 04:05:37PM -0800, Chandler Carruth wrote:
>> > On Thu, Dec 27, 2012 at 4:01 PM, Joerg Sonnenberger <joerg at britannica.bec.de
>> > > wrote:
>> >
>> > > On Thu, Dec 27, 2012 at 01:49:06PM -0500, Howard Hinnant wrote:
>> > > > > @@ -4583,7 +4584,7 @@
>> > > > >  string
>> > > > >  __time_get_storage<char>::__analyze(char fmt, const ctype<char>& ct)
>> > > > >  {
>> > > > > -    tm t = {0};
>> > > > > +    tm t = {0,0,0,0,0,0,0,0,0,0,0};
>> > > > >      t.tm_sec = 59;
>> > > > >      t.tm_min = 55;
>> > > > >      t.tm_hour = 23;
>> > > > > @@ -4729,7 +4730,7 @@
>> > > > >  wstring
>> > > > >  __time_get_storage<wchar_t>::__analyze(char fmt, const
>> > > ctype<wchar_t>& ct)
>> > > > >  {
>> > > > > -    tm t = {0};
>> > > > > +    tm t = {0,0,0,0,0,0,0,0,0,0,0};
>> > > > >      t.tm_sec = 59;
>> > > > >      t.tm_min = 55;
>> > > > >      t.tm_hour = 23;
>> > > >
>> > > > Rejected.  tm contains *at least* 9 int data members.  The portable and
>> > > > concise way to zero initialize this struct is with the single {0}.
>> > >
>> > > What about using memset as alternative?
>> >
>> >
>> > Why? The compiler warning from GCC is simply wrong. The other members are
>> > in fact initialized. I see no reason to use a more verbose construct which
>> > requires a declaration of an external function merely to avoid a patently
>> > incorrect warning.
>> 
>> The compiler warning from GCC is for the (common) case that a field is
>> added later and so the initializer depends only on the default value of
>> the field. That warning is helpful for plain C, but not so much for C++
>> data structures, where constructures serve the purpose of preserving API
>> compatibility.
>> 
> There are common patterns that are pretty well established in both C and C++, that should not trigger such a warning. I think '= {0}' is one such pattern...

<philosophical>

Every time a warning goes into a language, the language is sub-setted.  And sub-setting a language is bad.  However sometimes we learn over time that language features are bad too.  And yet removing language features is very difficult and takes decades under the best of circumstances.  And so an engineering judgement has to be made:  Is the language feature so bad that it would be better to sub-set it?

Compiler writers carry a heavy burden in making this call.  And they are people too (capable of failing to make the right choice).  And sometimes their warnings are aiming at truly bad features and accidentally target not-bad features.

At the end of the day, there needs to be a feedback loop on the warnings that compiler writers write.  Otherwise one gets a unstable system where compiler writers continue to write more and more restrictive warnings on a language, sub-setting it to the point it is no longer recognizable to the language designers, nor the skilled users of that language.  Such feedback is best if it comes from the users of the language and of the compiler.

This is my feedback:

    tm t = {0};

is the best way I know of for zeroing a C struct.  Especially an opaque one.  It is in no way ambiguous, confusing, inefficient, or error prone.  Indeed this style prevents errors, is concise, and is very efficient.  It is well-defined by the language in a portable fashion.

    **  Warning on it is a compiler bug.  Push back there.  **

This isn't the first time I've considered a warning a bug, nor will it be the last.  Nevertheless our languages aren't perfect either, and I continue to value the compiler writer's effort to supply warnings for situations that are likely to be errors.

</philosophical>

Howard




More information about the cfe-commits mailing list