Skip to content

Commit 1d0bdf0

Browse files
Use unique_ptr for HDF proxy factory
Change DataObjectRepository to accept ownership via std::unique_ptr for HDF proxy factories and deprecate the raw-pointer setter. Update SWIG bindings to support std::unique_ptr when available (SWIG >= 4.1) while preserving compatibility paths for older SWIG versions and adjust C#/Java typemaps accordingly. Also add MSVC compile flag /Zc:__cplusplus in src/CMakeLists.txt (see discussion https://gitlab.kitware.com/cmake/cmake/-/work_items/18837#note_722476).
1 parent 1505267 commit 1d0bdf0

8 files changed

Lines changed: 59 additions & 25 deletions

File tree

CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
3737
set(CMAKE_CXX_EXTENSIONS OFF)
3838
set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
3939

40-
4140
# ============================================================================
4241
# checking for required dependencies
4342
# ============================================================================

example/example.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2609,7 +2609,7 @@ bool serialize(const string& filePath)
26092609
epcDoc.serializeFrom(repo);
26102610

26112611
// Check HDF proxy factory
2612-
repo.setHdfProxyFactory(new HdfProxyFactoryExample());
2612+
repo.setHdfProxyFactory(std::make_unique<HdfProxyFactoryExample>());
26132613
auto* exoticHdfPox = repo.createHdfProxy("", "Dummy Exotic HDF proxy", "", "", COMMON_NS::DataObjectRepository::openingMode::READ_ONLY);
26142614
double dummyPoints[3] = { 1.0, 2.0, 3.0 };
26152615
repo.createPointSetRepresentation("", "")->pushBackXyzGeometryPatch(1, dummyPoints, exoticHdfPox);

example/exampleMPI.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void serialize(const std::string filename) {
5050
COMMON_NS::EpcDocument pck(filePath);
5151
COMMON_NS::DataObjectRepository repo;
5252

53-
repo.setHdfProxyFactory(new COMMON_NS::HdfProxyMPIFactory());
53+
repo.setHdfProxyFactory(std::make_unique<COMMON_NS::HdfProxyMPIFactory>());
5454
EML2_NS::AbstractHdfProxy* hdfProxy = repo.createHdfProxy("", "Parallel Hdf Proxy", pck.getStorageDirectory(), pck.getName() + ".h5", COMMON_NS::DataObjectRepository::openingMode::OVERWRITE);
5555
repo.setDefaultHdfProxy(hdfProxy);
5656

@@ -91,7 +91,7 @@ void deserialize(const std::string filename) {
9191
if (world_rank == 0) {
9292
COMMON_NS::EpcDocument pck(filePath);
9393
COMMON_NS::DataObjectRepository repo;
94-
repo.setHdfProxyFactory(new COMMON_NS::HdfProxyMPIFactory());
94+
repo.setHdfProxyFactory(std::make_unique<COMMON_NS::HdfProxyMPIFactory>());
9595
const std::string resqmlResult = pck.deserializeInto(repo);
9696
if (!resqmlResult.empty()) {
9797
std::cerr << resqmlResult << std::endl;

src/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,12 @@ set (EML_PREFIX_2_3 "eml2_3")
4242
# Define the compile options according to the compiler
4343
include(${FESAPI_ROOT_DIR}/cmake/cmake-harden.cmake)
4444
harden(${CPP_LIBRARY_NAME} CXX RUNTIME)
45+
# About /Zc:__cplusplus see https://gitlab.kitware.com/cmake/cmake/-/work_items/18837#note_722476
4546
target_compile_options(${CPP_LIBRARY_NAME} PRIVATE
4647
$<$<CXX_COMPILER_ID:MSVC>:/bigobj>
4748
$<$<CXX_COMPILER_ID:MSVC>:/MP>
4849
$<$<CXX_COMPILER_ID:MSVC>:/W4>
50+
$<$<CXX_COMPILER_ID:MSVC>:/Zc:__cplusplus>
4951
)
5052
if (WITH_RESQML2_2)
5153
target_compile_definitions(${CPP_LIBRARY_NAME} PUBLIC "-DWITH_RESQML2_2")

src/common/DataObjectRepository.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3597,13 +3597,20 @@ void DataObjectRepository::registerDataFeeder(COMMON_NS::DataFeeder * dataFeeder
35973597
dataFeeders.push_back(dataFeeder);
35983598
}
35993599

3600-
void DataObjectRepository::setHdfProxyFactory(COMMON_NS::HdfProxyFactory * factory) {
3600+
void DataObjectRepository::setHdfProxyFactory(COMMON_NS::HdfProxyFactory* factory) {
36013601
if (factory == nullptr) {
36023602
throw invalid_argument("You cannot set a NULL HDF proxy factory.");
36033603
}
36043604
hdfProxyFactory.reset(factory);
36053605
}
36063606

3607+
void DataObjectRepository::setHdfProxyFactory(std::unique_ptr<COMMON_NS::HdfProxyFactory> factory) {
3608+
if (factory == nullptr) {
3609+
throw invalid_argument("You cannot set a NULL HDF proxy factory.");
3610+
}
3611+
hdfProxyFactory = std::move(factory);
3612+
}
3613+
36073614
COMMON_NS::AbstractObject* DataObjectRepository::resolvePartial(COMMON_NS::AbstractObject * partialObj)
36083615
{
36093616
for (auto const* dataFeeder : dataFeeders) {

src/common/DataObjectRepository.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,21 @@ namespace COMMON_NS
303303
* @param[in] target The target object of the relationship.
304304
*/
305305
DLL_IMPORT_OR_EXPORT void deleteRelationship(COMMON_NS::AbstractObject * source, COMMON_NS::AbstractObject * target);
306-
306+
307307
/**
308308
* Set the factory used to create HDF proxy and takes ownership of this HDF Proxy factory (don't delete it!)
309309
*
310310
* @param[in] factory If non-null, the factory.
311311
*/
312-
DLL_IMPORT_OR_EXPORT void setHdfProxyFactory(COMMON_NS::HdfProxyFactory * factory);
312+
[[deprecated("Use setHdfProxyFactory(std::unique_ptr<COMMON_NS::HdfProxyFactory> factory) instead.")]]
313+
DLL_IMPORT_OR_EXPORT void setHdfProxyFactory(COMMON_NS::HdfProxyFactory* factory);
314+
315+
/**
316+
* Set the factory used to create HDF proxy and takes ownership of this HDF Proxy factory (don't use it after having called this method!)
317+
*
318+
* @param[in] factory If non-null, the factory.
319+
*/
320+
DLL_IMPORT_OR_EXPORT void setHdfProxyFactory(std::unique_ptr<COMMON_NS::HdfProxyFactory> factory);
313321

314322
/**
315323
* Get the journal of the DataObject repository.

src/common/EpcDocument.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ string EpcDocument::deserializeInto(DataObjectRepository & repo, DataObjectRepos
315315
if (rel.getType().compare("http://schemas.energistics.org/package/2012/relationships/externalResource") == 0) {
316316
target = rel.getTarget();
317317
if (target.find("http://") == 0 || target.find("https://") == 0) {
318-
repo.setHdfProxyFactory(new HdfProxyROS3Factory());
318+
repo.setHdfProxyFactory(std::make_unique<HdfProxyROS3Factory>());
319319
}
320320
wrapper = repo.addOrReplaceGsoapProxy(package->extractFile(contentTypeEntry.second.getExtensionOrPartName().substr(1)), contentType, filePath);
321321
break;
@@ -411,10 +411,10 @@ std::string EpcDocument::deserializePartiallyInto(DataObjectRepository & repo, D
411411
if (allRels[relIndex].getType().compare("http://schemas.energistics.org/package/2012/relationships/externalResource") == 0) {
412412
const std::string target = allRels[relIndex].getTarget();
413413
if (target.find("http://") == 0 || target.find("https://") == 0) {
414-
repo.setHdfProxyFactory(new HdfProxyROS3Factory());
414+
repo.setHdfProxyFactory(std::make_unique<HdfProxyROS3Factory>());
415415
}
416416
else {
417-
repo.setHdfProxyFactory(new HdfProxyFactory());
417+
repo.setHdfProxyFactory(std::make_unique<HdfProxyFactory>());
418418
}
419419
wrapper = repo.addOrReplaceGsoapProxy(package->extractFile(it.second.getExtensionOrPartName().substr(1)), contentType, filePath);
420420
static_cast<EML2_0_NS::HdfProxy*>(wrapper)->setRelativePath(target);

swig/swigModule.i

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,11 @@ under the License.
1919
%module fesapi
2020
%feature("python:annotations", "c");
2121

22-
%{
23-
#if defined(__clang__) || defined(_MSC_VER)
24-
#elif defined(__GNUC__) || defined(__GNUG__)
25-
#pragma GCC diagnostic ignored "-Wcast-qual"
26-
#pragma GCC diagnostic ignored "-Wstrict-aliasing"
27-
#endif
28-
%}
29-
3022
%include "stdint.i"
3123
%include "std_string.i"
24+
#if SWIG_VERSION >= 0x040100
25+
%include <std_unique_ptr.i>
26+
#endif
3227

3328
%include "../src/nsDefinitions.h"
3429

@@ -94,13 +89,15 @@ under the License.
9489
SWIG_CSBODY_PROXY(public, public, SWIGTYPE)
9590
SWIG_CSBODY_TYPEWRAPPER(public, public, public, SWIGTYPE)
9691

92+
#if SWIG_VERSION < 0x040100
9793
%typemap(cscode) COMMON_NS::DataObjectRepository %{
9894
private HdfProxyFactory hdfProxyFactoryReference = null;
9995
%}
10096

10197
%typemap(csin,
10298
post=" hdfProxyFactoryReference = $csinput;"
10399
) COMMON_NS::HdfProxyFactory * factory "HdfProxyFactory.getCPtr($csinput)"
100+
#endif
104101

105102
%include "swigCsInclude.i"
106103
#endif
@@ -116,6 +113,7 @@ under the License.
116113
%}
117114
%include "swigPythonInclude.i"
118115

116+
#if SWIG_VERSION < 0x040100
119117
/* Following extensions aims at preventing the Python garbage collector from
120118
garbage collecting an HDF Proxy factory that may be still used by a repository. */
121119
%fragment("hdf_proxy_factory_reference_init", "init") {
@@ -133,6 +131,7 @@ under the License.
133131
%}
134132
}
135133
#endif
134+
#endif
136135

137136

138137
//************************/
@@ -220,6 +219,7 @@ namespace EML2_NS
220219
%module(directors="1") fesapi
221220
%feature("director") COMMON_NS::HdfProxyFactory;
222221

222+
#if SWIG_VERSION < 0x040100
223223
/*
224224
Following csbody and javabody typemap definitions force the target language to not
225225
manage the memory of the C++ part of wrapped COMMON_NS::HdfProxyFactory and
@@ -254,6 +254,7 @@ destructor).
254254
return (obj == null) ? 0 : obj.swigCPtr;
255255
}
256256
%}
257+
#endif
257258

258259
/*
259260
AbstractHdfProxy may be inherited in target languages. In such case, overriding
@@ -276,6 +277,10 @@ warning (205) during the Swig processing.
276277
return (obj == null) ? new global::System.Runtime.InteropServices.HandleRef(null, global::System.IntPtr.Zero) : obj.swigCPtr;
277278
}
278279
%}
280+
281+
#if SWIG_VERSION >= 0x040100
282+
%unique_ptr(COMMON_NS::HdfProxyFactory)
283+
#endif
279284
namespace COMMON_NS
280285
{
281286
%nodefaultctor; // Disable creation of default constructors
@@ -917,19 +922,22 @@ import java.lang.AutoCloseable;
917922
import com.f2i_consulting.fesapi.*;
918923
%}
919924
%typemap(javainterfaces) COMMON_NS::DataObjectRepository "AutoCloseable"
920-
%typemap(javacode) COMMON_NS::DataObjectRepository %{
921-
private HdfProxyFactory hdfProxyFactoryReference;
922-
925+
%typemap(javacode) COMMON_NS::DataObjectRepository %{
923926
@Override
924927
public void close() {
925928
clear();
926929
delete();
927930
}
928931
%}
932+
933+
#if SWIG_VERSION < 0x040100
934+
%typemap(javacode) COMMON_NS::DataObjectRepository %{
935+
private HdfProxyFactory hdfProxyFactoryReference;
936+
%}
929937
%typemap(javain,
930938
post=" hdfProxyFactoryReference = $javainput;"
931939
) COMMON_NS::HdfProxyFactory * factory "HdfProxyFactory.getCPtr($javainput)"
932-
940+
#endif
933941
/**
934942
* @brief A DataObjectRepository stores in memory all dataObjects.
935943
* This is the in-memory container which holds deserialized (EPC) files and fetched ETP dataobjects.
@@ -1039,13 +1047,23 @@ import com.f2i_consulting.fesapi.*;
10391047
* @param[in] hdfProxy If non-null, the HDF5 file proxy.
10401048
*/
10411049
void setDefaultHdfProxy(EML2_NS::AbstractHdfProxy* hdfProxy);
1042-
1050+
1051+
#if SWIG_VERSION >= 0x040100
1052+
/**
1053+
* Set the factory used to create HDF proxy and takes ownership of this HDF Proxy factory (don't use it after having called this method!)
1054+
*
1055+
* @param[in] factory If non-null, the factory.
1056+
*/
1057+
void setHdfProxyFactory(std::unique_ptr<COMMON_NS::HdfProxyFactory> factory);
1058+
#else
10431059
/**
1044-
* Set the factory used to create HDF proxy and takes ownership of this HDF Proxy factory (don't delete it!)
1060+
* Set the factory used to create HDF proxy and takes ownership of this HDF Proxy factory.
1061+
* Don't delete it! Do not allow gabage to collect it. Disown it from Swig since it will be destroyed by the C++ layer anyhow.
10451062
*
10461063
* @param[in] factory If non-null, the factory.
10471064
*/
1048-
void setHdfProxyFactory(COMMON_NS::HdfProxyFactory * factory);
1065+
void setHdfProxyFactory(COMMON_NS::HdfProxyFactory* factory);
1066+
#endif
10491067

10501068
/**
10511069
* Adds or replaces (based on Energistics XML definition) a data object in the repository. It

0 commit comments

Comments
 (0)