[MOAB-dev] r4016 - MOAB/trunk/tools/dagmc

[Jason Kraftcheck]

Paul Wilson wrote:
> Hi Steve,
> 
> Thanks again for finding this, but the more I think about it, I wonder
> if this is the right solutions.
> 
> It is theoretically possible to build MOAB/DAGMC apps with no reference
> to CGM whatsoever.  Under those circumstances, RefEntity will never be
> defined beyond the single declaration in DagMC.hpp.  Is this OK? 
> Wouldn't it be better to set the CGM macro in moab.make for inclusion in
> other apps?
> 

Requiring applications to provide pre-processor macros to use a library
header is a little bit of an API faux pas.  If a #define must be provided
for a header then it should be specified via an included header.  But that
shouldn't be necessary in this case.  If you remove the '#ifdef CGM' guards
and leave the guarded code, everything should compile fine regardless of
whether or not CGM is used.

- jason



> Paul
> 
> sjackson at cae.wisc.edu wrote:
>> Author: sjackson
>> Date: 2010-06-23 14:44:46 -0500 (Wed, 23 Jun 2010)
>> New Revision: 4016
>>
>> Modified:
>>    MOAB/trunk/tools/dagmc/DagMC.hpp
>> Log:
>> Fix a very old, but recently symptomatic, bug in DagMC.
>>
>> When MOAB is built with CGM, the macro "CGM" is defined.  Until now,
>> this caused an extra data member to be included in the DagMC class.
>> However, client code compiling against DagMC does not define the CGM
>> macro (the macro is not exported to moab.make).  This means a client
>> compiling against DagMC.hpp can wind up with an incorrect map of the
>> data members in a DagMC object.
>>
>> This hasn't surfaced as a problem before because the conditionally-
>> included data member was declared *after* all other client-facing
>> data members.  The reordering of data members in r4006 exposed the
>> problem.
>>
>> The easy solution (which I've taken) is to simply remove the checks
>> from the header file.  If CGM is not available, the extra declarations
>> are harmless (and DagMC gets one extra unused std::vector).
>>
>> Moral: Beware of #ifdefs in public headers, particularly if they
>> check for macros that client code may not know about.
>>
>> Modified: MOAB/trunk/tools/dagmc/DagMC.hpp
>> ===================================================================
>> --- MOAB/trunk/tools/dagmc/DagMC.hpp    2010-06-22 18:15:50 UTC (rev
>> 4015)
>> +++ MOAB/trunk/tools/dagmc/DagMC.hpp    2010-06-23 19:44:46 UTC (rev
>> 4016)
>> @@ -12,9 +12,7 @@
>>  
>>  #include "moab/OrientedBoxTreeTool.hpp"
>>  
>> -#ifdef CGM
>>  class RefEntity;
>> -#endif
>>  
>>  namespace moab {
>>  
>> @@ -349,10 +347,8 @@
>>      // entity index (contiguous 1-N indices) indexed like rootSets are
>>    std::vector<int> entIndices;
>>  
>> -#ifdef CGM
>>      // corresponding geometric entities indexed like rootSets are
>>    std::vector<RefEntity *> geomEntities;
>> -#endif
>>       // metadata
>>    Tag matTag, densTag, bcTag, impTag, tallyTag;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>   
> 


-- 
"A foolish consistency is the hobgoblin of little minds" - Ralph Waldo Emerson