[mpich-devel] [mpich-commits] [mpich] MPICH primary repository branch, master, updated. v3.2a1-34-geda105b

[Dave Goodell (dgoodell)]

On Oct 1, 2014, at 10:20 PM, Junchao Zhang <jczhang at mcs.anl.gov> wrote:

> On Wed, Oct 1, 2014 at 5:40 PM, Dave Goodell (dgoodell) <dgoodell at cisco.com>
> wrote:
> 
>> Begin forwarded message:
>> 
>>> - Log -----------------------------------------------------------------
>>> 
>> http://git.mpich.org/mpich.git/commitdiff/eda105b029572ebf7f4008f6de2e1afb9ccf6fc2
>>> 
>>> commit eda105b029572ebf7f4008f6de2e1afb9ccf6fc2
>>> Author: Junchao Zhang <jczhang at mcs.anl.gov>
>>> Date:   Tue Sep 16 15:58:55 2014 -0500
>>> 
>>>   Remove unnessary array init in MPIU_CHKLMEM_DECL
>>> 
>>>   Initializing mpiu_chklmem_stk_sp_ to 0 should already have the effect
>> to
>>>   initialize mpiu_chklmem_stk_[n_].
>>> 
>>>   Signed-off-by: Antonio J. Pena <apenya at mcs.anl.gov>
>>> 
>>> diff --git a/src/include/mpimem.h b/src/include/mpimem.h
>>> index a217e8b..e1478a5 100644
>>> --- a/src/include/mpimem.h
>>> +++ b/src/include/mpimem.h
>>> @@ -317,7 +317,7 @@ extern char *strdup( const char * );
>>> }}
>>> #else
>>> #define MPIU_CHKLMEM_DECL(n_) \
>>> - void *(mpiu_chklmem_stk_[n_]) = {0};\
>>> + void *(mpiu_chklmem_stk_[n_]); \
>>> int mpiu_chklmem_stk_sp_=0;\
>>> MPIU_AssertDeclValue(const int mpiu_chklmem_stk_sz_,n_)
>> 
>> You might want to revert this.  My recollection is that not all compilers
>> are smart enough to see that mpiu_chklmem_stk_ is not used before being
>> defined, which can lead to a *lot* of warnings throughout the MPICH
>> codebase.
>> 
> 
> Dave, I don't quite understand your concern. I removed the array
> initialization code since MPIU_CHKLMEM_FREEALL() can only free pointers set
> before. The number of pointers is mpiu_chklmem_stk_sp_, which is
> initialized to 0.

Look, I don't have time (or frankly, that much interest) to run down an example compiler where this will happen, but I definitely made this change in the past because we were encountering the warning: http://git.mpich.org/mpich.git/commitdiff/70f4742

I wish I had made a better commit message at the time :-/

I have a suspicion that GCC ~4.1 with some set of (non-?)optimization flags would issue the "may be used uninitialized" warning, but I really don't recall for sure.  I don't disagree with your analysis that the value cannot actually be used uninitialized, I'm just disputing whether the compiler will issue a warning about it.

Actually, it looks like I'm not the only person who has experienced this, Toni fixed an issue like this much more recently than 2009: http://git.mpich.org/mpich.git/commitdiff/d8b8eb6

>> Also, what's the sudden interest in restoring C89 support again?  The
>> whole world really does support enough of C99 now...  Seems like a big
>> waste of time and an unnecessary annoyance to developers who might want to
>> use useful C99 features (e.g., named structure initializers).
>> 
>> 
> See https://trac.mpich.org/projects/mpich/ticket/2167


That's not really an answer.  It's just a rehash of the endless discussion that MPICH development has had for the past 7+ years.

Open MPI and many other projects now require nominal C99 compliance for any recent release.  All the work that you did to run around and fix the "--enable-strict=c89" mode was just a huge waste of time.  The correct fix is to delete "--enable-strict=c89" and embrace the "future" of 6+ years ago.

-Dave