Fix windows MSVC build. This CL improves the build experience with MSVC: - It adds the build flag '/bigobj' otherwise the build of the unit test fails. - It adds the flag '/wd4267' to suppress signed / unsigned int conversions (size_t to int). - It removes the use of std::aligned_storage from FixedArray. This has been done from the Abseil Team and is in the absl::FixedArray. Those changes has been ported to ceres. This fixes the alignemnt for older MSVC versions due to a bug in the implementation of std::aligned_storage, and prevents the use of the macro '_ENABLE_EXTENDED_ALIGNED_STORAGE' for newer MSVC versions, which is problematic as it could affect user code. - Fix of the fixed array unit test. Due to the use of std::tuple instead of absl::tuple in ceres::internal::FixedArray, the unit test needs to reflect that change as well. - Replaces 'add_definitions' with 'add_compile_options' for compiler flags as suggested by the cmake documentation. Change-Id: I63f08cd6c0a8db8c9931289b909b4deafd75b039
diff --git a/CMakeLists.txt b/CMakeLists.txt index b25cee3..f11e579 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt
@@ -527,25 +527,30 @@ # [1] https://msdn.microsoft.com/en-us/library/4hwaceh6.aspx add_definitions("-D_USE_MATH_DEFINES") # Disable signed/unsigned int conversion warnings. - add_definitions("/wd4018") + add_compile_options("/wd4018" "/wd4267") # Disable warning about using struct/class for the same symobl. - add_definitions("/wd4099") + add_compile_options("/wd4099") # Disable warning about the insecurity of using "std::copy". - add_definitions("/wd4996") + add_compile_options("/wd4996") # Disable performance warning about int-to-bool conversion. - add_definitions("/wd4800") + add_compile_options("/wd4800") # Disable performance warning about fopen insecurity. - add_definitions("/wd4996") + add_compile_options("/wd4996") # Disable warning about int64 to int32 conversion. Disabling # this warning may not be correct; needs investigation. # TODO(keir): Investigate these warnings in more detail. - add_definitions("/wd4244") + add_compile_options("/wd4244") # It's not possible to use STL types in DLL interfaces in a portable and # reliable way. However, that's what happens with Google Log and Google Flags # on Windows. MSVC gets upset about this and throws warnings that we can't do # much about. The real solution is to link static versions of Google Log and # Google Test, but that seems tricky on Windows. So, disable the warning. - add_definitions("/wd4251") + add_compile_options("/wd4251") + + # Add bigobj flag otherwise the build would fail due to large object files + # probably resulting from generated headers (like the fixed-size schur + # specializations). + add_compile_options("/bigobj") # Google Flags doesn't have their DLL import/export stuff set up correctly, # which results in linker warnings. This is irrelevant for Ceres, so ignore
diff --git a/include/ceres/internal/fixed_array.h b/include/ceres/internal/fixed_array.h index c107dfc..15481f3 100644 --- a/include/ceres/internal/fixed_array.h +++ b/include/ceres/internal/fixed_array.h
@@ -163,8 +163,7 @@ CopyRange(storage_.alloc(), storage_.begin(), first, last); } - // Releases any resources. - ~FixedArray() { + ~FixedArray() noexcept { for (auto* cur = storage_.begin(); cur != storage_.end(); ++cur) { AllocatorTraits::destroy(storage_.alloc(), cur); } @@ -358,7 +357,7 @@ // error: call to int __builtin___sprintf_chk(etc...) // will always overflow destination buffer [-Werror] // - template <typename OuterT = value_type, + template <typename OuterT, typename InnerT = typename std::remove_extent<OuterT>::type, size_t InnerN = std::extent<OuterT>::value> struct StorageElementWrapper { @@ -369,9 +368,6 @@ typename std::conditional<std::is_array<value_type>::value, StorageElementWrapper<value_type>, value_type>::type; - using StorageElementBuffer = - typename std::aligned_storage<sizeof(StorageElement), - alignof(StorageElement)>::type; static pointer AsValueType(pointer ptr) { return ptr; } static pointer AsValueType(StorageElementWrapper<value_type>* ptr) { @@ -381,25 +377,25 @@ static_assert(sizeof(StorageElement) == sizeof(value_type), ""); static_assert(alignof(StorageElement) == alignof(value_type), ""); - struct NonEmptyInlinedStorage { - StorageElement* data() { - return reinterpret_cast<StorageElement*>(inlined_storage_.data()); - } + class NonEmptyInlinedStorage { + public: + StorageElement* data() { return reinterpret_cast<StorageElement*>(buff_); } + void AnnotateConstruct(size_type) {} + void AnnotateDestruct(size_type) {} // #ifdef ADDRESS_SANITIZER // void* RedzoneBegin() { return &redzone_begin_; } // void* RedzoneEnd() { return &redzone_end_ + 1; } // #endif // ADDRESS_SANITIZER - void AnnotateConstruct(size_type) {} - void AnnotateDestruct(size_type) {} - + private: // ADDRESS_SANITIZER_REDZONE(redzone_begin_); - std::array<StorageElementBuffer, inline_elements> inlined_storage_; + alignas(StorageElement) char buff_[sizeof(StorageElement[inline_elements])]; // ADDRESS_SANITIZER_REDZONE(redzone_end_); }; - struct EmptyInlinedStorage { + class EmptyInlinedStorage { + public: StorageElement* data() { return nullptr; } void AnnotateConstruct(size_type) {} void AnnotateDestruct(size_type) {} @@ -460,6 +456,13 @@ Storage storage_; }; +template <typename T, size_t N, typename A> +constexpr size_t FixedArray<T, N, A>::kInlineBytesDefault; + +template <typename T, size_t N, typename A> +constexpr typename FixedArray<T, N, A>::size_type + FixedArray<T, N, A>::inline_elements; + } // namespace internal } // namespace ceres
diff --git a/internal/ceres/fixed_array_test.cc b/internal/ceres/fixed_array_test.cc index 79ae511..95cba7f 100644 --- a/internal/ceres/fixed_array_test.cc +++ b/internal/ceres/fixed_array_test.cc
@@ -477,8 +477,8 @@ // Simulate the data members of ceres::internal::FixedArray, a pointer and a // size_t. struct Data { + std::tuple<size_t, std::allocator<double>> size_alloc_; TooBig* p; - size_t size; }; // Make sure TooBig objects are not inlined for 0 or default size.