[MOAB-dev] SEEK_SET again

[Jed Brown]

Jason Kraftcheck wrote:
> Jed Brown wrote:
>> Jason Kraftcheck wrote:
>>> Jed Brown wrote:
>>>
>>>  IIRC, your concern when I brought this up before was that some user
>>>> might expect that behavior when they included the header
>>>> indirectly.  In
>>>> my opinion, it's none of MOAB's business to make these sort of
>>>> decisions
>>>> for the user.  If it is causing a problem for a user, they should
>>>> include mpi.h themselves so that they can control it's environment.
>>>>
>>> I agree that MOAB should not make such decisions for the user.  That
>>> is why
>>> we do things the way that we do.
>>>
>>> And if they include mpi.h before MBProcConfig.hpp then parsing of
>>> MBProcConfig will fail because those constants are defined,
>>> regardless of
>>> whether or not we do #define MPICH_IGNORE_CXX_SEEK in
>>> MBProcConfig.hpp.  And
>>> if the application includes mpi.h after MBProcConfig, then
>>> MPICH_IGNORE_CXX_SEEK will be defined for the mpi.h included by that
>>> application regardless of what the application expects.
>>
>> Uh, this is the current state, but not the case if you stripped out the
>> macro smashing.  Suppose MBProcConfig.hpp just did a plain
>>
>>   #include <mpi.h>
>>
>> (not wrapped, no reference to SEEK_*).  Now the user can include mpi.h
>> in any order with respect to MOAB headers.  If the MPI implementation is
>> obscure and ignores the issue, and they have already included something
>> that defined these macros, they will get a compiler error.  Now the
>> users options are to either smash the macros themselves or define a
>> macro so the MPI doesn't try to define the MPI::SEEK_* constants.
>> Either solution will work fine and does not involve MOAB doing anything
>> special.
> 
> MOAB will not compile in that case, because the MPI constants conflict
> with the fcntl.h macros.  Doing nothing is not an option.

MOAB does not use the MPI constants so it is only a problem if the MPI
implementation does not know how to deal with the macros being defined
when mpi.h is included.  Since all MPI implementations consider it to be
a bug in the spec, they have to deal with this, usually by changing the
macros to global constants/enums, or by providing a way to not define
the MPI constants.  What I'm saying is that the MOAB workaround is
conflicting with better handling in almost every mpi.h.

>> The problem case is if they define the macros after including MPI and
>> then use MPI::SEEK_* in their code (which the preprocessor will replace
>> with something like MPI::0 causing unexpected behavior).  The present
>> macro smashing forces this behavior on the user despite more intelligent
>> handling by the MPI (unless the user manages to ensure that those macros
>> are not defined when MOAB includes mpi.h, thus circumventing that
>> wrapping).  (MPICH2 replaces the stdio macros with enums, Open MPI
>> replaces them with static const int which is friendlier in the presence
>> of templates, both solutions are better than leaving them as macros
>> which produces surprising runtime behavior.)
>>
> 
> Yes, the whole idea of naming constants SEEK_* in mpi.h is broken.  For
> a sufficient number versions of mpich and other MPI 2.0 implementations,
> there is no correct solution that compiles in all cases.  The best we
> can do is make things work for the case when noone uses the broken
> constants, which we do.  Doing nothing will result in a broken MOAB
> build, and defining MPICH_IGNORE_CXX_SEEK will break things for
> applications using MOAB just as much as what we do now.

Doing nothing works fine unless the MPI implementation didn't provide
their own workaround, which all the recent releases have.  Defining
MPICH_IGNORE_CXX_SEEK is only a problem if the user uses MPI::SEEK_* in
their own code (instead of MPI_SEEK_*), which is already broken (due to
incorrect runtime behavior) if SEEK_* are macros (this is why mpi.h will
redefine them to not be macros).

The current solution does not compile with Open MPI, is suboptimal with
recent MPICH2, and is worse than defining MPICH_IGNORE_CXX_SEEK for
older MPICH because it turns an obvious compile-time error into a
subtle platform-dependent runtime error.

Ultimately I have no say in the matter, so I'll shut up about this.


Jed

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.mcs.anl.gov/pipermail/moab-dev/attachments/20091021/5fec957d/attachment.pgp>