<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
<title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
Alas, I find myself more convinced of the others' view. <br>
<br>
It seems dangerous for the public interface to depend on a define, thus
allowing the library to be built with one interface and the calling app
to be built expecting another. (Perhaps in an earlier email to me)
Steve pointed out that moab.make is more of a convenience than a
requirement for building apps with MOAB. We could include relevant
defines in moab.make (one way or another) but it still allows for the
possibility that this inconsistency arises when a developer decides not
to use moab.make. <br>
<br>
Furthermore, it was a particularly difficult inconsistency to track
down as there were no compile-, link-, or runtime errors - just the
wrong answer when the code ran!<br>
<br>
Note, also, that CGM defines are still available after this fix, they
just don't change the public interface.<br>
<br>
Some other ideas:<br>
<ul>
<li>Modify the header (eg. DagMC.hpp) at install-time to install a
header that is unconditionally the same as the conditions used in
building MOAB. That is, parse the header at install time with the same
macros as used to build so that the interface is consistent and
unambiguous<br>
</li>
<li><begin kludge (I'm brainstorming here)>Declare a function
prototype and/or invoke a function within the conditional code that
will at least raise a link-time error when the inconsistency
arises</kludge></li>
</ul>
Paul<br>
<br>
<br>
<br>
Tim Tautges wrote:
<blockquote cite="mid:4C2296A1.1050608@mcs.anl.gov" type="cite">I agree
with Paul, if MOAB is compiled with CGM on, the CGM defines etc. should
be available to client apps, probably via inclusion of cgm.make in
moab.make.
<br>
<br>
Jim, could you investigate? Thanks.
<br>
<br>
- tim
<br>
<br>
On 06/23/2010 04:19 PM, Paul Wilson wrote:
<br>
<blockquote type="cite">Hi Steve,
<br>
<br>
Thanks again for finding this, but the more I think about it, I wonder
<br>
if this is the right solutions.
<br>
<br>
It is theoretically possible to build MOAB/DAGMC apps with no reference
<br>
to CGM whatsoever. Under those circumstances, RefEntity will never be
<br>
defined beyond the single declaration in DagMC.hpp. Is this OK?
Wouldn't
<br>
it be better to set the CGM macro in moab.make for inclusion in other
apps?
<br>
<br>
Paul
<br>
<br>
<a class="moz-txt-link-abbreviated" href="mailto:sjackson@cae.wisc.edu">sjackson@cae.wisc.edu</a> wrote:
<br>
<blockquote type="cite">Author: sjackson
<br>
Date: 2010-06-23 14:44:46 -0500 (Wed, 23 Jun 2010)
<br>
New Revision: 4016
<br>
<br>
Modified:
<br>
MOAB/trunk/tools/dagmc/DagMC.hpp
<br>
Log:
<br>
Fix a very old, but recently symptomatic, bug in DagMC.
<br>
<br>
When MOAB is built with CGM, the macro "CGM" is defined. Until now,
<br>
this caused an extra data member to be included in the DagMC class.
<br>
However, client code compiling against DagMC does not define the CGM
<br>
macro (the macro is not exported to moab.make). This means a client
<br>
compiling against DagMC.hpp can wind up with an incorrect map of the
<br>
data members in a DagMC object.
<br>
<br>
This hasn't surfaced as a problem before because the conditionally-
<br>
included data member was declared *after* all other client-facing
<br>
data members. The reordering of data members in r4006 exposed the
<br>
problem.
<br>
<br>
The easy solution (which I've taken) is to simply remove the checks
<br>
from the header file. If CGM is not available, the extra declarations
<br>
are harmless (and DagMC gets one extra unused std::vector).
<br>
<br>
Moral: Beware of #ifdefs in public headers, particularly if they
<br>
check for macros that client code may not know about.
<br>
<br>
Modified: MOAB/trunk/tools/dagmc/DagMC.hpp
<br>
===================================================================
<br>
--- MOAB/trunk/tools/dagmc/DagMC.hpp 2010-06-22 18:15:50 UTC (rev 4015)
<br>
+++ MOAB/trunk/tools/dagmc/DagMC.hpp 2010-06-23 19:44:46 UTC (rev 4016)
<br>
@@ -12,9 +12,7 @@
<br>
<br>
#include "moab/OrientedBoxTreeTool.hpp"
<br>
<br>
-#ifdef CGM
<br>
class RefEntity;
<br>
-#endif
<br>
<br>
namespace moab {
<br>
<br>
@@ -349,10 +347,8 @@
<br>
// entity index (contiguous 1-N indices) indexed like rootSets are
<br>
std::vector<int> entIndices;
<br>
<br>
-#ifdef CGM
<br>
// corresponding geometric entities indexed like rootSets are
<br>
std::vector<RefEntity *> geomEntities;
<br>
-#endif
<br>
// metadata
<br>
Tag matTag, densTag, bcTag, impTag, tallyTag;
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Paul Wilson
-- ------------------------------------------------------------------ --
Paul P.H. Wilson 419 Engineering Research Building
<a class="moz-txt-link-abbreviated" href="mailto:wilsonp@engr.wisc.edu">wilsonp@engr.wisc.edu</a> 1500 Engineering Dr
ph/fax: 608/263-0807 Madison, WI 53706
My calendar: <a class="moz-txt-link-freetext" href="http://bit.ly/pphw-calendar">http://bit.ly/pphw-calendar</a>
Computational Nuclear Engineering Research Group
<a class="moz-txt-link-freetext" href="http://cnerg.engr.wisc.edu">http://cnerg.engr.wisc.edu</a>
Associate Professor, Nuclear Engineering
Engineering Physics Department
<a class="moz-txt-link-freetext" href="http://www.engr.wisc.edu/ep">http://www.engr.wisc.edu/ep</a>
Chair, Energy Analysis & Policy Certificate
Nelson Institute for Environmental Studies
<a class="moz-txt-link-freetext" href="http://nelson.wisc.edu/eap/">http://nelson.wisc.edu/eap/</a>
Contributing to the joy and improvement
of all those around me</pre>
</body>
</html>