From 21aa7a2e856c535b0aeaf3643032e94b12e5465d Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Tue, 17 Jan 2017 10:12:00 +0000 Subject: [PATCH 1/5] Fixed unrwapping of scalar references. --- wrap/Argument.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index 52d9ca0b5..a47c6711c 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -75,16 +75,17 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { string cppType = type.qualifiedName("::"); string matlabUniqueType = type.qualifiedName(); + bool isNotScalar = !Argument::isScalar(); if (is_ptr && type.category != Qualified::EIGEN) // A pointer: emit an "unwrap_shared_ptr" call which returns a pointer file.oss << "boost::shared_ptr<" << cppType << "> " << name << " = unwrap_shared_ptr< "; - else if (is_ref && type.category != Qualified::EIGEN) + else if (is_ref && isNotScalar && type.category != Qualified::EIGEN) // A reference: emit an "unwrap_shared_ptr" call and de-reference the pointer file.oss << cppType << "& " << name << " = *unwrap_shared_ptr< "; else - // Not a pointer or a reference: emit an "unwrap" call + // Not a pointer, or a reference to a scalar type. Therefore, emit an "unwrap" call // unwrap is specified in matlab.h as a series of template specializations // that know how to unpack the expected MATLAB object // example: double tol = unwrap< double >(in[2]); @@ -92,7 +93,7 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { file.oss << cppType << " " << name << " = unwrap< "; file.oss << cppType << " >(" << matlabName; - if( (is_ptr || is_ref) && type.category != Qualified::EIGEN) + if( (is_ptr || is_ref) && isNotScalar && type.category != Qualified::EIGEN) file.oss << ", \"ptr_" << matlabUniqueType << "\""; file.oss << ");" << endl; } From f50c3c0d51fa7f4d015ec84e5a536a0cc4ffd918 Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Tue, 17 Jan 2017 16:53:28 +0000 Subject: [PATCH 2/5] Use INSTALL_NAME_DIR to embed names in the dylibs and avoid linker errors.y --- cmake/GtsamMatlabWrap.cmake | 3 ++- gtsam/CMakeLists.txt | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmake/GtsamMatlabWrap.cmake b/cmake/GtsamMatlabWrap.cmake index a9e04a01a..3165a307a 100644 --- a/cmake/GtsamMatlabWrap.cmake +++ b/cmake/GtsamMatlabWrap.cmake @@ -70,7 +70,8 @@ function(wrap_library_internal interfaceHeader linkLibraries extraIncludeDirs ex set(mexModuleExt mexglx) endif() elseif(APPLE) - set(mexModuleExt mexmaci64) + set(mexModuleExt mexmaci64) + set(CMAKE_INSTALL_DIR_NAME ${GTSAM_TOOLBOX_INSTALL_PATH}) elseif(MSVC) if(CMAKE_CL_64) set(mexModuleExt mexw64) diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 8c1d8bb43..63528d3b4 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -113,6 +113,10 @@ if (GTSAM_BUILD_STATIC_LIBRARY) PREFIX "lib" COMPILE_DEFINITIONS GTSAM_IMPORT_STATIC) endif() + if(APPLE) # Set the + set_target_properties(gtsam PROPERTIES + INSTALL_DIR_NAME ${CMAKE_INSTALL_PREFIX}/lib) + endif() install(TARGETS gtsam EXPORT GTSAM-exports ARCHIVE DESTINATION lib) list(APPEND GTSAM_EXPORTED_TARGETS gtsam) set(GTSAM_EXPORTED_TARGETS "${GTSAM_EXPORTED_TARGETS}" PARENT_SCOPE) From d8d7c5618a11076a1a56a7e3232d4e195f70b1c8 Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Thu, 19 Jan 2017 01:49:12 +0000 Subject: [PATCH 3/5] Generate an error and exit if trying to wrap a non-const scalar reference. --- wrap/Argument.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index a47c6711c..6badf7794 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -77,6 +77,12 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { string matlabUniqueType = type.qualifiedName(); bool isNotScalar = !Argument::isScalar(); + // We cannot handle scalar non const references + if (!isNotScalar && is_ref && !is_const) { + cerr << "Cannot wrap a scalar non-const reference" << endl; + exit(-1); + } + if (is_ptr && type.category != Qualified::EIGEN) // A pointer: emit an "unwrap_shared_ptr" call which returns a pointer file.oss << "boost::shared_ptr<" << cppType << "> " << name From d1422ac9216af6fa53b49932cb258a195c1b22ba Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Thu, 19 Jan 2017 09:28:04 +0000 Subject: [PATCH 4/5] Reverted change to make files to make the pull request clean. --- cmake/GtsamMatlabWrap.cmake | 3 +-- gtsam/CMakeLists.txt | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/cmake/GtsamMatlabWrap.cmake b/cmake/GtsamMatlabWrap.cmake index 3165a307a..a9e04a01a 100644 --- a/cmake/GtsamMatlabWrap.cmake +++ b/cmake/GtsamMatlabWrap.cmake @@ -70,8 +70,7 @@ function(wrap_library_internal interfaceHeader linkLibraries extraIncludeDirs ex set(mexModuleExt mexglx) endif() elseif(APPLE) - set(mexModuleExt mexmaci64) - set(CMAKE_INSTALL_DIR_NAME ${GTSAM_TOOLBOX_INSTALL_PATH}) + set(mexModuleExt mexmaci64) elseif(MSVC) if(CMAKE_CL_64) set(mexModuleExt mexw64) diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 63528d3b4..8c1d8bb43 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -113,10 +113,6 @@ if (GTSAM_BUILD_STATIC_LIBRARY) PREFIX "lib" COMPILE_DEFINITIONS GTSAM_IMPORT_STATIC) endif() - if(APPLE) # Set the - set_target_properties(gtsam PROPERTIES - INSTALL_DIR_NAME ${CMAKE_INSTALL_PREFIX}/lib) - endif() install(TARGETS gtsam EXPORT GTSAM-exports ARCHIVE DESTINATION lib) list(APPEND GTSAM_EXPORTED_TARGETS gtsam) set(GTSAM_EXPORTED_TARGETS "${GTSAM_EXPORTED_TARGETS}" PARENT_SCOPE) From 6a109aca9bae0b858099f59d084a0254841397ac Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Fri, 20 Jan 2017 01:58:59 +0000 Subject: [PATCH 5/5] Throw an exception rather than call exit. --- wrap/Argument.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index 6badf7794..01da3a756 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -79,8 +79,7 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { // We cannot handle scalar non const references if (!isNotScalar && is_ref && !is_const) { - cerr << "Cannot wrap a scalar non-const reference" << endl; - exit(-1); + throw std::runtime_error("Cannot unwrap a scalar non-const reference"); } if (is_ptr && type.category != Qualified::EIGEN)