diff --git a/cmake/test_config/CMakeLists.txt b/cmake/test_config/CMakeLists.txt index 06f17a5156..7abeb298a6 100644 --- a/cmake/test_config/CMakeLists.txt +++ b/cmake/test_config/CMakeLists.txt @@ -2,6 +2,7 @@ project(Test_DAGMC_config) cmake_minimum_required(VERSION 2.8) set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}) +set(CMAKE_CXX_STANDARD 11) find_package(DAGMC REQUIRED) diff --git a/news/PR-0671.rst b/news/PR-0671.rst new file mode 100644 index 0000000000..d1889289cc --- /dev/null +++ b/news/PR-0671.rst @@ -0,0 +1,23 @@ +**Added:** None + +**Changed:** +- DagMC: updated pointer management to RAII ("Resource Allocation Is + Initialization") technique: + - MBI is now a shared_ptr unless passed as a raw pointer in the DagMC + constructor (can be returned as a shared_ptr if not provided as a raw + pointer) + - GTT is now a shared_ptr, and can only be returned as such + - GQT is now a uniq_ptr, (and can't be return - not change there) +- DagMC tests: + - DagMC instance is now a shared_ptr + - when used MBI instance is now a shared_ptr + +**Deprecated:** +- DagMC: Deprecated constructor using a raw pointer for the MBI instance, + prefered way uses shared_ptr for MBI instance. + +**Removed:** None + +**Fixed:** None + +**Security:** None diff --git a/src/dagmc/DagMC.cpp b/src/dagmc/DagMC.cpp index 6fd0a0a856..76919858e3 100644 --- a/src/dagmc/DagMC.cpp +++ b/src/dagmc/DagMC.cpp @@ -39,20 +39,35 @@ const bool counting = false; /* controls counts of ray casts and pt_in_vols */ const std::map DagMC::no_synonyms; // DagMC Constructor -DagMC::DagMC(Interface* mb_impl, double overlap_tolerance, double p_numerical_precision) { +DagMC::DagMC(std::shared_ptr mb_impl, double overlap_tolerance, double p_numerical_precision) { moab_instance_created = false; // if we arent handed a moab instance create one - if (NULL == mb_impl) { - mb_impl = new moab::Core(); + if (nullptr == mb_impl) { + mb_impl = std::make_shared(); moab_instance_created = true; } + MBI_shared_ptr = mb_impl; + // set the internal moab pointer + MBI = MBI_shared_ptr.get(); + + // make new GeomTopoTool and GeomQueryTool + GTT = std::make_shared (MBI, false); + GQT = std::unique_ptr(new GeomQueryTool(GTT.get(), overlap_tolerance, p_numerical_precision)); + + // This is the correct place to uniquely define default values for the dagmc settings + defaultFacetingTolerance = .001; +} + +DagMC::DagMC(Interface* mb_impl, double overlap_tolerance, double p_numerical_precision) { + moab_instance_created = false; // set the internal moab pointer MBI = mb_impl; + MBI_shared_ptr = nullptr; // make new GeomTopoTool and GeomQueryTool - GTT = new moab::GeomTopoTool(MBI, false); - GQT = new moab::GeomQueryTool(GTT, overlap_tolerance, p_numerical_precision); + GTT = std::make_shared (MBI, false); + GQT = std::unique_ptr(new GeomQueryTool(GTT.get(), overlap_tolerance, p_numerical_precision)); // This is the correct place to uniquely define default values for the dagmc settings defaultFacetingTolerance = .001; @@ -60,15 +75,11 @@ DagMC::DagMC(Interface* mb_impl, double overlap_tolerance, double p_numerical_pr // Destructor DagMC::~DagMC() { - // delete the GeomTopoTool and GeomQueryTool - delete GTT; - delete GQT; // if we created the moab instance // clear it if (moab_instance_created) { MBI->delete_mesh(); - delete MBI; } } diff --git a/src/dagmc/DagMC.hpp b/src/dagmc/DagMC.hpp index 1cc8ee6ed2..5bb8ef2412 100644 --- a/src/dagmc/DagMC.hpp +++ b/src/dagmc/DagMC.hpp @@ -12,10 +12,12 @@ #include "moab/GeomQueryTool.hpp" #include "DagMCVersion.hpp" -#include +#include #include +#include +#include #include -#include +#include class RefEntity; @@ -57,7 +59,10 @@ class CartVect; class DagMC { public: // Constructor - DagMC(Interface* mb_impl = NULL, double overlap_tolerance = 0., double numerical_precision = .001); + DagMC(std::shared_ptr mb_impl = nullptr, double overlap_tolerance = 0., double numerical_precision = .001); + // Deprecated Constructor + [[ deprecated("Replaced by DagMC(std::shared_ptr mb_impl, ... )") ]] + DagMC(Interface* mb_impl, double overlap_tolerance = 0., double numerical_precision = .001); // Destructor ~DagMC(); @@ -365,7 +370,7 @@ class DagMC { public: OrientedBoxTreeTool* obb_tree() {return GTT->obb_tree();} - GeomTopoTool* geom_tool() {return GTT;} + std::shared_ptr geom_tool() {return GTT;} ErrorCode write_mesh(const char* ffile, const int flen); @@ -382,16 +387,25 @@ class DagMC { /** Get the instance of MOAB used by functions in this file. */ Interface* moab_instance() {return MBI;} + std::shared_ptr moab_instance_sptr() { + if (nullptr == MBI_shared_ptr) + std::runtime_error("MBI instance is not defined as a shared pointer !"); + return MBI_shared_ptr; + } private: /* PRIVATE MEMBER DATA */ + // Shared_ptr owning *MBI (if allocated internally) + std::shared_ptr MBI_shared_ptr; + // Use for the call to MOAB interface, should never be deleted in the DagMC instanced + // MBI is either externally owned or owned by the MBI_shared_ptr Interface* MBI; bool moab_instance_created; - GeomTopoTool* GTT; - GeomQueryTool* GQT; + std::shared_ptr GTT; + std::unique_ptr GQT; public: Tag nameTag, facetingTolTag; diff --git a/src/dagmc/tests/dagmc_pointinvol_test.cpp b/src/dagmc/tests/dagmc_pointinvol_test.cpp index 443c7849fa..314ad10c60 100644 --- a/src/dagmc/tests/dagmc_pointinvol_test.cpp +++ b/src/dagmc/tests/dagmc_pointinvol_test.cpp @@ -10,7 +10,7 @@ using namespace moab; using moab::DagMC; -moab::DagMC* DAG; +std::shared_ptr DAG; static const char input_file[] = "test_geom.h5m"; @@ -18,7 +18,7 @@ class DagmcPointInVolTest : public ::testing::Test { protected: virtual void SetUp() { // Create new DAGMC instance - DAG = new DagMC(); + DAG = std::make_shared(); // Load mesh from file rloadval = DAG->load_file(input_file); assert(rloadval == moab::MB_SUCCESS); @@ -26,9 +26,7 @@ class DagmcPointInVolTest : public ::testing::Test { rval = DAG->init_OBBTree(); assert(rval == moab::MB_SUCCESS); } - virtual void TearDown() { - delete DAG; - } + virtual void TearDown() {} protected: moab::ErrorCode rloadval; moab::ErrorCode rval; diff --git a/src/dagmc/tests/dagmc_rayfire_test.cpp b/src/dagmc/tests/dagmc_rayfire_test.cpp index d771a158ea..caec3a2622 100644 --- a/src/dagmc/tests/dagmc_rayfire_test.cpp +++ b/src/dagmc/tests/dagmc_rayfire_test.cpp @@ -11,7 +11,7 @@ using namespace moab; using moab::DagMC; -moab::DagMC* DAG; +std::shared_ptr DAG; static const char input_file[] = "test_geom.h5m"; double eps = 1.0e-6; @@ -20,7 +20,7 @@ class DagmcRayFireTest : public ::testing::Test { protected: virtual void SetUp() { // Create new DAGMC instance - DAG = new DagMC(); + DAG = std::make_shared(); // Load mesh from file rloadval = DAG->load_file(input_file); assert(rloadval == moab::MB_SUCCESS); @@ -28,9 +28,7 @@ class DagmcRayFireTest : public ::testing::Test { rval = DAG->init_OBBTree(); assert(rval == moab::MB_SUCCESS); } - virtual void TearDown() { - delete DAG; - } + virtual void TearDown() {} protected: moab::ErrorCode rloadval; moab::ErrorCode rval; diff --git a/src/dagmc/tests/dagmc_simple_test.cpp b/src/dagmc/tests/dagmc_simple_test.cpp index df7cc79ee5..a013c835d9 100644 --- a/src/dagmc/tests/dagmc_simple_test.cpp +++ b/src/dagmc/tests/dagmc_simple_test.cpp @@ -29,18 +29,15 @@ TEST_F(DagmcSimpleTest, dagmc_load_file) { TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc) { /* 1 - Test with external moab, load file in DAGMC*/ // make new moab core - Core* mbi = new moab::Core(); + std::shared_ptr mbi = std::make_shared(); // make new dagmc into that moab - DagMC* dagmc = new moab::DagMC(mbi); + std::shared_ptr dagmc = std::make_shared(mbi); ErrorCode rval; + // load a file rval = dagmc->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); - - // delete dagmc - delete dagmc; - delete mbi; } TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_via_moab) { @@ -48,16 +45,12 @@ TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_via_moab) { // load the file into moab rather than dagmc ErrorCode rval; - moab::Core* mbi = new moab::Core(); + std::shared_ptr mbi = std::make_shared(); rval = mbi->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); - moab::DagMC* dagmc = new moab::DagMC(mbi); + std::shared_ptr dagmc = std::make_shared(mbi); rval = dagmc->load_existing_contents(); EXPECT_EQ(rval, MB_SUCCESS); - - // delete dagmc; - delete dagmc; - delete mbi; } TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_internal) { @@ -65,11 +58,10 @@ TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_internal) { // make new dagmc into that moab ErrorCode rval; - moab::DagMC* dagmc = new moab::DagMC(); + std::shared_ptr dagmc = std::make_shared(); // load a file rval = dagmc->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); - delete dagmc; } TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_build_obb) { @@ -77,18 +69,15 @@ TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_build_obb) { // make new moab core ErrorCode rval; - moab::Core* mbi = new moab::Core(); + std::shared_ptr mbi = std::make_shared(); // make new dagmc into that moab - DagMC* dagmc = new moab::DagMC(mbi); + std::shared_ptr dagmc = std::make_shared(mbi); // load a file rval = dagmc->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); rval = dagmc->init_OBBTree(); EXPECT_EQ(rval, MB_SUCCESS); - // delete dagmc - delete dagmc; - delete mbi; } TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_via_moab_build_obb) { @@ -96,18 +85,14 @@ TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_via_moab_build_obb) { // load the file into moab rather than dagmc ErrorCode rval; - moab::Core* mbi = new moab::Core(); + std::shared_ptr mbi = std::make_shared(); rval = mbi->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); - moab::DagMC* dagmc = new moab::DagMC(mbi); + std::shared_ptr dagmc = std::make_shared(mbi); rval = dagmc->load_existing_contents(); EXPECT_EQ(rval, MB_SUCCESS); rval = dagmc->init_OBBTree(); EXPECT_EQ(rval, MB_SUCCESS); - - // delete dagmc; - delete dagmc; - delete mbi; } TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_internal_build_obb) { @@ -115,20 +100,19 @@ TEST_F(DagmcSimpleTest, dagmc_load_file_dagmc_internal_build_obb) { // make new dagmc into that moab ErrorCode rval; - moab::DagMC* dagmc = new moab::DagMC(); + std::shared_ptr dagmc = std::make_shared(); // load a file rval = dagmc->load_file(input_file); EXPECT_EQ(rval, MB_SUCCESS); rval = dagmc->init_OBBTree(); EXPECT_EQ(rval, MB_SUCCESS); - delete dagmc; } TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval) { // make new dagmc std::cout << "test_obb_retreval" << std::endl; - DagMC* dagmc = new moab::DagMC(); + std::shared_ptr dagmc = std::make_shared(); ErrorCode rval; // load a file @@ -140,10 +124,7 @@ TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval) { // write the file rval = dagmc->write_mesh("fcad", 4); - // now remove the dagmc instance a - delete dagmc; - - dagmc = new moab::DagMC(); + dagmc.reset(new DagMC()); rval = dagmc->load_file("fcad"); EXPECT_EQ(rval, MB_SUCCESS); rval = dagmc->init_OBBTree(); @@ -151,7 +132,6 @@ TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval) { // delete the fcad file remove("fcad"); - delete dagmc; } TEST_F(DagmcSimpleTest, dagmc_build_obb) { @@ -180,7 +160,7 @@ TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval_rayfire) { // make new dagmc std::cout << "test_obb_retreval and ray_fire" << std::endl; - DagMC* dagmc = new moab::DagMC(); + std::shared_ptr dagmc = std::make_shared(); ErrorCode rval; // load a file @@ -192,11 +172,8 @@ TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval_rayfire) { // write the file rval = dagmc->write_mesh("fcad", 4); - // now remove the dagmc instance - delete dagmc; - // now create new DAGMC - dagmc = new moab::DagMC(); + dagmc.reset(new DagMC()); rval = dagmc->load_file("fcad"); EXPECT_EQ(rval, MB_SUCCESS); rval = dagmc->init_OBBTree(); @@ -220,7 +197,6 @@ TEST_F(DagmcSimpleTest, dagmc_test_obb_retreval_rayfire) { rval = DAG->ray_fire(vol_h, xyz, dir, next_surf, next_surf_dist); EXPECT_EQ(rval, MB_SUCCESS); EXPECT_NEAR(expect_next_surf_dist, next_surf_dist, eps); - delete dagmc; } TEST_F(DagmcSimpleTest, dagmc_rayfire) { diff --git a/src/dagmc/tests/dagmc_unit_tests.cpp b/src/dagmc/tests/dagmc_unit_tests.cpp index 0180195200..8b96488e5b 100644 --- a/src/dagmc/tests/dagmc_unit_tests.cpp +++ b/src/dagmc/tests/dagmc_unit_tests.cpp @@ -8,10 +8,10 @@ #include // dagmc instance -moab::DagMC* DAG; +std::shared_ptr DAG; // metadata instance -dagmcMetaData* dgm; +std::shared_ptr dgm; //---------------------------------------------------------------------------// // TEST FIXTURES @@ -24,7 +24,7 @@ class DagmcMetadataTest : public ::testing::Test { // Default h5m file for testing std::string infile = "test_dagmc.h5m"; - DAG = new moab::DagMC(); + DAG = std::make_shared(); rloadval = DAG->load_file(infile.c_str()); assert(rloadval == moab::MB_SUCCESS); @@ -34,10 +34,7 @@ class DagmcMetadataTest : public ::testing::Test { assert(rval == moab::MB_SUCCESS); } - virtual void TearDown() { - delete DAG; - delete dgm; - } + virtual void TearDown() {} protected: @@ -59,7 +56,7 @@ TEST_F(DagmcMetadataTest, SetUp) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestMatAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -93,7 +90,7 @@ TEST_F(DagmcMetadataTest, TestMatAssigns) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestDensityAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -119,7 +116,7 @@ TEST_F(DagmcMetadataTest, TestDensityAssigns) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestMatDensityAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -149,7 +146,7 @@ TEST_F(DagmcMetadataTest, TestMatDensityAssigns) { TEST_F(DagmcMetadataTest, TestUnpackString) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -174,7 +171,7 @@ TEST_F(DagmcMetadataTest, TestUnpackString) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestImportanceAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -201,7 +198,7 @@ TEST_F(DagmcMetadataTest, TestImportanceAssigns) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestBoundaryAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -230,7 +227,7 @@ TEST_F(DagmcMetadataTest, TestBoundaryAssigns) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestTallyAssigns) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); @@ -259,7 +256,7 @@ TEST_F(DagmcMetadataTest, TestTallyAssigns) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestReturnProperty) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); std::string return_string = ""; return_string = dgm->return_property("mat:Steel", "mat", ":", false); @@ -296,7 +293,7 @@ TEST_F(DagmcMetadataTest, TestReturnProperty) { //---------------------------------------------------------------------------// TEST_F(DagmcMetadataTest, TestSplitString) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); std::string to_split = "Neutron/1.0"; std::pair pair = dgm->split_string(to_split, "/"); @@ -322,7 +319,7 @@ TEST_F(DagmcMetadataTest, TestSplitString) { // test to make sure the function try_to_make_int works TEST_F(DagmcMetadataTest, TestTryToMakeInt) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); EXPECT_EQ(dgm->try_to_make_int("1"), true); EXPECT_EQ(dgm->try_to_make_int("1A"), false); @@ -339,7 +336,7 @@ class DagmcMetadataTestImplCompMat : public ::testing::Test { // Default h5m file for testing std::string infile = "test_dagmc_impl.h5m"; - DAG = new moab::DagMC(); + DAG = std::make_shared(); rloadval = DAG->load_file(infile.c_str()); assert(rloadval == moab::MB_SUCCESS); @@ -349,10 +346,7 @@ class DagmcMetadataTestImplCompMat : public ::testing::Test { assert(rval == moab::MB_SUCCESS); } - virtual void TearDown() { - // delete dgm; - delete DAG; - } + virtual void TearDown() {} protected: @@ -372,7 +366,7 @@ TEST_F(DagmcMetadataTestImplCompMat, SetUp) { // is set TEST_F(DagmcMetadataTestImplCompMat, ImplCompMat) { // new metadata instance - dgm = new dagmcMetaData(DAG); + dgm = std::make_shared(DAG.get()); // process dgm->load_property_data(); // loop over the volumes diff --git a/src/dagmc/tools/pt_vol_test.cpp b/src/dagmc/tools/pt_vol_test.cpp index bd95f2290f..1118003c36 100644 --- a/src/dagmc/tools/pt_vol_test.cpp +++ b/src/dagmc/tools/pt_vol_test.cpp @@ -73,7 +73,7 @@ int main(int argc, char* argv[]) { << " of geometry " << filename << std::endl; - DagMC dagmc = DagMC(); + DagMC dagmc{}; rval = dagmc.load_file(filename); if (MB_SUCCESS != rval) { std::cerr << "Failed to load file." << std::endl; diff --git a/src/dagmc/tools/ray_fire_test.cpp b/src/dagmc/tools/ray_fire_test.cpp index 0926f6a020..ddd909b75a 100644 --- a/src/dagmc/tools/ray_fire_test.cpp +++ b/src/dagmc/tools/ray_fire_test.cpp @@ -243,7 +243,7 @@ int main(int argc, char* argv[]) { trv_stats = new OrientedBoxTreeTool::TrvStats; } - DagMC dagmc = DagMC(); + DagMC dagmc{}; rval = dagmc.load_file(filename); if (MB_SUCCESS != rval) { std::cerr << "Failed to load file '" << filename << "'" << std::endl;