Fix MSVC compilation errors
Disable the definition of `min`/`max` macros by defining `NOMINMAX`
and prevent macro substitution in the public interface.
Also, quiet floating-point comparisons are defined as template functions
by the MSVC STL which causes compilation errors due to ambiguities in
resolving the template parameter types.
Fixes #668
Fixes #716
Fixes #718
Change-Id: I5fe7832a6a3a7ad0421a2557527528c34b88e9c7
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c218882..4fac3ba 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -444,7 +444,8 @@
set_ceres_threading_model("${CERES_THREADING_MODEL}")
if (BUILD_BENCHMARKS)
- find_package(benchmark QUIET)
+ # Version 1.3 was first to provide import targets
+ find_package(benchmark 1.3 QUIET)
if (benchmark_FOUND)
message("-- Found Google benchmark library. Building Ceres benchmarks.")
else()
@@ -612,6 +613,15 @@
$<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:GNU>>:-Wno-deprecated-declarations>
$<$<CXX_COMPILER_ID:MSVC>:/wd4996>)
+if (CMAKE_VERSION VERSION_LESS 3.12)
+ # Disable the definiton of min/max macros within the project
+ if (WIN32)
+ add_definitions (-DNOMINMAX)
+ endif (WIN32)
+else (CMAKE_VERSION VERSION_LESS 3.12)
+ add_compile_definitions($<$<BOOL:${WIN32}>:NOMINMAX>)
+endif (CMAKE_VERSION VERSION_LESS 3.12)
+
# Configure the Ceres config.h compile options header using the current
# compile options and put the configured header into the Ceres build
# directory. Note that the ceres/internal subdir in <build>/config where
diff --git a/include/ceres/cubic_interpolation.h b/include/ceres/cubic_interpolation.h
index 134bad2..3ca6b11 100644
--- a/include/ceres/cubic_interpolation.h
+++ b/include/ceres/cubic_interpolation.h
@@ -191,7 +191,7 @@
}
EIGEN_STRONG_INLINE void GetValue(const int n, double* f) const {
- const int idx = std::min(std::max(begin_, n), end_ - 1) - begin_;
+ const int idx = (std::min)((std::max)(begin_, n), end_ - 1) - begin_;
if (kInterleaved) {
for (int i = 0; i < kDataDimension; ++i) {
f[i] = static_cast<double>(data_[kDataDimension * idx + i]);
@@ -402,9 +402,9 @@
EIGEN_STRONG_INLINE void GetValue(const int r, const int c, double* f) const {
const int row_idx =
- std::min(std::max(row_begin_, r), row_end_ - 1) - row_begin_;
+ (std::min)((std::max)(row_begin_, r), row_end_ - 1) - row_begin_;
const int col_idx =
- std::min(std::max(col_begin_, c), col_end_ - 1) - col_begin_;
+ (std::min)((std::max)(col_begin_, c), col_end_ - 1) - col_begin_;
const int n = (kRowMajor) ? num_cols_ * row_idx + col_idx
: num_rows_ * col_idx + row_idx;
diff --git a/include/ceres/internal/householder_vector.h b/include/ceres/internal/householder_vector.h
index e5fc9bf..7700208 100644
--- a/include/ceres/internal/householder_vector.h
+++ b/include/ceres/internal/householder_vector.h
@@ -82,11 +82,11 @@
v->head(v->rows() - 1) /= v_pivot;
}
-template <typename XVectorType, typename Scalar, int N>
-Eigen::Matrix<Scalar, N, 1> ApplyHouseholderVector(
+template <typename XVectorType, typename Derived>
+typename Derived::PlainObject ApplyHouseholderVector(
const XVectorType& y,
- const Eigen::Matrix<Scalar, N, 1>& v,
- const Scalar& beta) {
+ const Eigen::MatrixBase<Derived>& v,
+ const typename Derived::Scalar& beta) {
return (y - v * (beta * (v.transpose() * y)));
}
diff --git a/include/ceres/internal/numeric_diff.h b/include/ceres/internal/numeric_diff.h
index c44733f..351845c 100644
--- a/include/ceres/internal/numeric_diff.h
+++ b/include/ceres/internal/numeric_diff.h
@@ -121,7 +121,7 @@
// thus ridders_relative_initial_step_size is used.
if (kMethod == RIDDERS) {
min_step_size =
- std::max(min_step_size, options.ridders_relative_initial_step_size);
+ (std::max)(min_step_size, options.ridders_relative_initial_step_size);
}
// For each parameter in the parameter block, use finite differences to
@@ -132,7 +132,7 @@
num_residuals_internal);
for (int j = 0; j < parameter_block_size_internal; ++j) {
- const double delta = std::max(min_step_size, step_size(j));
+ const double delta = (std::max)(min_step_size, step_size(j));
if (kMethod == RIDDERS) {
if (!EvaluateRiddersJacobianColumn(functor,
@@ -296,7 +296,7 @@
// norm_error is supposed to decrease as the finite difference tableau
// generation progresses, serving both as an estimate for differentiation
// error and as a measure of differentiation numerical stability.
- double norm_error = std::numeric_limits<double>::max();
+ double norm_error = (std::numeric_limits<double>::max)();
// Loop over decreasing step sizes until:
// 1. Error is smaller than a given value (ridders_epsilon),
@@ -342,7 +342,7 @@
options.ridders_step_shrink_factor;
// Compute the difference between the previous value and the current.
- double candidate_error = std::max(
+ double candidate_error = (std::max)(
(current_candidates->col(k) - current_candidates->col(k - 1))
.norm(),
(current_candidates->col(k) - previous_candidates->col(k - 1))
diff --git a/include/ceres/internal/port.h b/include/ceres/internal/port.h
index 2bc44f4..4275b0e 100644
--- a/include/ceres/internal/port.h
+++ b/include/ceres/internal/port.h
@@ -63,4 +63,26 @@
// 202002L)
#endif // !defined(CERES_HAS_CPP20)
+// Prevents symbols from being substituted by the corresponding macro definition
+// under the same name. For instance, min and max are defined as macros on
+// Windows (unless NOMINMAX is defined) which causes compilation errors when
+// defining or referencing symbols under the same name.
+//
+// To be robust in all cases particularly when NOMINMAX cannot be used, use this
+// macro to annotate min/max declarations/definitions. Examples:
+//
+// int max CERES_PREVENT_MACRO_SUBSTITUTION();
+// min CERES_PREVENT_MACRO_SUBSTITUTION(a, b);
+// max CERES_PREVENT_MACRO_SUBSTITUTION(a, b);
+//
+// NOTE: In case the symbols for which the substitution must be prevented are
+// used within another macro, the substitution must be inhibited using parens as
+//
+// (std::numerical_limits<double>::max)()
+//
+// since the helper macro will not work here. Do not use this technique in
+// general case, because it will prevent argument-dependent lookup (ADL).
+//
+#define CERES_PREVENT_MACRO_SUBSTITUTION // Yes, it's empty
+
#endif // CERES_PUBLIC_INTERNAL_PORT_H_
diff --git a/include/ceres/jet.h b/include/ceres/jet.h
index 10ce51e..fba1e2a 100644
--- a/include/ceres/jet.h
+++ b/include/ceres/jet.h
@@ -443,15 +443,9 @@
using std::fpclassify;
using std::hypot;
using std::isfinite;
-using std::isgreater;
-using std::isgreaterequal;
using std::isinf;
-using std::isless;
-using std::islessequal;
-using std::islessgreater;
using std::isnan;
using std::isnormal;
-using std::isunordered;
using std::log;
using std::log10;
using std::log1p;
@@ -465,6 +459,47 @@
using std::tan;
using std::tanh;
+// MSVC (up to 1930) defines quiet comparison functions as template functions
+// which causes compilation errors due to ambiguity in the template parameter
+// type resolution for using declarations in the ceres namespace. Workaround the
+// issue by defining specific overload and bypass MSVC standard library
+// definitions.
+#if defined(_MSC_VER)
+inline bool isgreater(double lhs,
+ double rhs) noexcept(noexcept(std::isgreater(lhs, rhs))) {
+ return std::isgreater(lhs, rhs);
+}
+inline bool isless(double lhs,
+ double rhs) noexcept(noexcept(std::isless(lhs, rhs))) {
+ return std::isless(lhs, rhs);
+}
+inline bool islessequal(double lhs,
+ double rhs) noexcept(noexcept(std::islessequal(lhs,
+ rhs))) {
+ return std::islessequal(lhs, rhs);
+}
+inline bool isgreaterequal(double lhs, double rhs) noexcept(
+ noexcept(std::isgreaterequal(lhs, rhs))) {
+ return std::isgreaterequal(lhs, rhs);
+}
+inline bool islessgreater(double lhs, double rhs) noexcept(
+ noexcept(std::islessgreater(lhs, rhs))) {
+ return std::islessgreater(lhs, rhs);
+}
+inline bool isunordered(double lhs,
+ double rhs) noexcept(noexcept(std::isunordered(lhs,
+ rhs))) {
+ return std::isunordered(lhs, rhs);
+}
+#else
+using std::isgreater;
+using std::isgreaterequal;
+using std::isless;
+using std::islessequal;
+using std::islessgreater;
+using std::isunordered;
+#endif
+
#ifdef CERES_HAS_CPP20
using std::lerp;
using std::midpoint;
@@ -1246,8 +1281,9 @@
static constexpr bool tinyness_before =
std::numeric_limits<T>::tinyness_before;
- static constexpr ceres::Jet<T, N> min() noexcept {
- return ceres::Jet<T, N>(std::numeric_limits<T>::min());
+ static constexpr ceres::Jet<T, N> min
+ CERES_PREVENT_MACRO_SUBSTITUTION() noexcept {
+ return ceres::Jet<T, N>((std::numeric_limits<T>::min)());
}
static constexpr ceres::Jet<T, N> lowest() noexcept {
return ceres::Jet<T, N>(std::numeric_limits<T>::lowest());
@@ -1271,8 +1307,9 @@
return ceres::Jet<T, N>(std::numeric_limits<T>::denorm_min());
}
- static constexpr ceres::Jet<T, N> max() noexcept {
- return ceres::Jet<T, N>(std::numeric_limits<T>::max());
+ static constexpr ceres::Jet<T, N> max
+ CERES_PREVENT_MACRO_SUBSTITUTION() noexcept {
+ return ceres::Jet<T, N>((std::numeric_limits<T>::max)());
}
};
@@ -1326,8 +1363,8 @@
};
};
- static inline Real highest() { return Real(std::numeric_limits<T>::max()); }
- static inline Real lowest() { return Real(-std::numeric_limits<T>::max()); }
+ static inline Real highest() { return Real((std::numeric_limits<T>::max)()); }
+ static inline Real lowest() { return Real(-(std::numeric_limits<T>::max)()); }
};
// Specifying the return type of binary operations between Jets and scalar types
diff --git a/include/ceres/manifold.h b/include/ceres/manifold.h
index 14a5792..458eec0 100644
--- a/include/ceres/manifold.h
+++ b/include/ceres/manifold.h
@@ -285,6 +285,9 @@
public:
ProductManifold(const ProductManifold&) = delete;
ProductManifold& operator=(const ProductManifold&) = delete;
+ // NOTE: Do not remove the trivial destructor as this will cause linker errors
+ // in MSVC builds.
+ ~ProductManifold() override;
// NOTE: The constructor takes ownership of the input
// manifolds.
@@ -307,7 +310,7 @@
ManifoldPtr& manifold = manifolds_[i];
manifold = std::move(manifolds_array[i]);
- buffer_size_ = std::max(
+ buffer_size_ = (std::max)(
buffer_size_, manifold->TangentSize() * manifold->AmbientSize());
ambient_size_ += manifold->AmbientSize();
tangent_size_ += manifold->TangentSize();
diff --git a/include/ceres/tiny_solver.h b/include/ceres/tiny_solver.h
index 16fd2a8..86a655d 100644
--- a/include/ceres/tiny_solver.h
+++ b/include/ceres/tiny_solver.h
@@ -250,7 +250,7 @@
const Scalar max_diagonal = 1e32;
for (int i = 0; i < lm_diagonal_.rows(); ++i) {
lm_diagonal_[i] = std::sqrt(
- u * std::min(std::max(jtj_(i, i), min_diagonal), max_diagonal));
+ u * (std::min)((std::max)(jtj_(i, i), min_diagonal), max_diagonal));
jtj_regularized_(i, i) += lm_diagonal_[i] * lm_diagonal_[i];
}
@@ -306,7 +306,7 @@
}
Scalar tmp = Scalar(2 * rho - 1);
- u = u * std::max(Scalar(1 / 3.), 1 - tmp * tmp * tmp);
+ u = u * (std::max)(Scalar(1 / 3.), Scalar(1) - tmp * tmp * tmp);
v = 2;
} else {
diff --git a/internal/ceres/CMakeLists.txt b/internal/ceres/CMakeLists.txt
index ea0bed8..fa538fe 100644
--- a/internal/ceres/CMakeLists.txt
+++ b/internal/ceres/CMakeLists.txt
@@ -287,7 +287,7 @@
# CERES_STATIC_DEFINE is generated by the GenerateExportHeader CMake module
# used to autogerate export.h. The macro should not be renamed without
# updating the corresponding generate_export_header invocation.
- target_compile_definitions(ceres_static PRIVATE CERES_STATIC_DEFINE)
+ target_compile_definitions(ceres_static PUBLIC CERES_STATIC_DEFINE)
else()
# In a static library build, not additional access layer is necessary as all
# symbols are visible.
@@ -399,14 +399,6 @@
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
if (BUILD_TESTING AND GFLAGS)
- add_library(test_util STATIC
- evaluator_test_utils.cc
- numeric_diff_test_utils.cc
- test_util.cc)
-
- target_include_directories(test_util PUBLIC ${Ceres_SOURCE_DIR}/internal)
- target_link_libraries (test_util PUBLIC ceres_static gflags)
-
add_library(gtest gmock_gtest_all.cc gmock_main.cc)
if (BUILD_SHARED_LIBS)
# Define gtest-specific shared library flags for compilation.
@@ -417,12 +409,21 @@
target_compile_definitions(gtest
PRIVATE GTEST_CREATE_SHARED_LIBRARY=1
GFLAGS_DLL_DEFINE_FLAG=GTEST_API_
- INTERFACE GTEST_LINKED_AS_SHARED_LIBRARY=1)
+ INTERFACE GTEST_LINKED_AS_SHARED_LIBRARY=1
+ GFLAGS_DLL_DECLARE_FLAG=GTEST_API_)
endif()
target_include_directories(gtest PRIVATE ${Ceres_SOURCE_DIR}/internal/ceres)
target_link_libraries(gtest PRIVATE Ceres::ceres gflags)
+ add_library(test_util STATIC
+ evaluator_test_utils.cc
+ numeric_diff_test_utils.cc
+ test_util.cc)
+
+ target_include_directories(test_util PUBLIC ${Ceres_SOURCE_DIR}/internal)
+ target_link_libraries (test_util PUBLIC ceres_static gflags gtest)
+
macro (CERES_TEST NAME)
add_executable(${NAME}_test ${NAME}_test.cc)
# Pull in local headers from the generated test directories when ceres_test()
diff --git a/internal/ceres/autodiff_benchmarks/CMakeLists.txt b/internal/ceres/autodiff_benchmarks/CMakeLists.txt
index 610ebc3..99af152 100644
--- a/internal/ceres/autodiff_benchmarks/CMakeLists.txt
+++ b/internal/ceres/autodiff_benchmarks/CMakeLists.txt
@@ -9,9 +9,3 @@
add_executable(autodiff_benchmarks autodiff_benchmarks.cc)
add_dependencies_to_benchmark(autodiff_benchmarks)
target_compile_options(autodiff_benchmarks PRIVATE ${CERES_BENCHMARK_FLAGS})
-
-# All other flags + fast-math
-list(APPEND CERES_BENCHMARK_FAST_MATH_FLAGS ${CERES_BENCHMARK_FLAGS} "-ffast-math")
-add_executable(autodiff_benchmarks_fast_math autodiff_benchmarks.cc)
-add_dependencies_to_benchmark(autodiff_benchmarks_fast_math)
-target_compile_options(autodiff_benchmarks_fast_math PRIVATE ${CERES_BENCHMARK_FAST_MATH_FLAGS})
diff --git a/internal/ceres/dense_cholesky_test.cc b/internal/ceres/dense_cholesky_test.cc
index 0e84207..c5189b8 100644
--- a/internal/ceres/dense_cholesky_test.cc
+++ b/internal/ceres/dense_cholesky_test.cc
@@ -97,19 +97,25 @@
}
}
-INSTANTIATE_TEST_SUITE_P(_,
- DenseCholeskyTest,
- ::testing::Values(EIGEN
+namespace {
+
+// NOTE: preprocessor directives in a macro are not standard conforming
+decltype(auto) MakeValues() {
+ return ::testing::Values(EIGEN
#ifndef CERES_NO_LAPACK
- ,
- LAPACK
+ ,
+ LAPACK
#endif
#ifndef CERES_NO_CUDA
- ,
- CUDA
+ ,
+ CUDA
#endif
- ),
- ParamInfoToString);
+ );
+}
+
+} // namespace
+
+INSTANTIATE_TEST_SUITE_P(_, DenseCholeskyTest, MakeValues(), ParamInfoToString);
} // namespace internal
} // namespace ceres
diff --git a/internal/ceres/jet_test.cc b/internal/ceres/jet_test.cc
index f06c599..6a76e1d 100644
--- a/internal/ceres/jet_test.cc
+++ b/internal/ceres/jet_test.cc
@@ -30,7 +30,11 @@
// The floating-point environment access and modification is only meaningful
// with the following pragma.
+#ifdef _MSC_VER
+#pragma fenv_access(on)
+#else
#pragma STDC FENV_ACCESS ON
+#endif
#include "ceres/jet.h"
diff --git a/internal/ceres/levenberg_marquardt_strategy_test.cc b/internal/ceres/levenberg_marquardt_strategy_test.cc
index 0e7ec8d..86d35a2 100644
--- a/internal/ceres/levenberg_marquardt_strategy_test.cc
+++ b/internal/ceres/levenberg_marquardt_strategy_test.cc
@@ -148,7 +148,7 @@
// are versions of glog which are not in the google namespace.
using namespace google;
-#if defined(_MSC_VER)
+#if defined(GLOG_NO_ABBREVIATED_SEVERITIES)
// Use GLOG_WARNING to support MSVC if GLOG_NO_ABBREVIATED_SEVERITIES
// is defined.
EXPECT_CALL(log,
diff --git a/internal/ceres/manifold.cc b/internal/ceres/manifold.cc
index 451286a..1f4781e 100644
--- a/internal/ceres/manifold.cc
+++ b/internal/ceres/manifold.cc
@@ -312,6 +312,20 @@
return true;
}
+// While the trivial destructor is not necessary, MSVC (1930) complains in
+// shared library builds by issuing a linker error:
+//
+// bundle_adjuster.obj : error LNK2019: unresolved external symbol
+// "__declspec(dllimport) const ceres::ProductManifold::`vftable'"
+// (__imp_??_7ProductManifold@ceres@@6B@) referenced in function "public:
+// __cdecl ceres::ProductManifold::ProductManifold<class
+// ceres::QuaternionManifold,class ceres::EuclideanManifold>(class
+// ceres::QuaternionManifold *,class ceres::EuclideanManifold *)"
+// (??$?0VQuaternionManifold@ceres@@VEuclideanManifold@1@@ProductManifold@ceres@@QEAA@PEAVQuaternionManifold@1@PEAVEuclideanManifold@1@@Z)
+//
+// Providing a trivial implementation here resolves the issue.
+ProductManifold::~ProductManifold() = default;
+
int ProductManifold::AmbientSize() const { return ambient_size_; }
int ProductManifold::TangentSize() const { return tangent_size_; }
diff --git a/internal/ceres/miniglog/glog/logging.h b/internal/ceres/miniglog/glog/logging.h
index f03c914..5604bd4 100644
--- a/internal/ceres/miniglog/glog/logging.h
+++ b/internal/ceres/miniglog/glog/logging.h
@@ -132,7 +132,7 @@
// added, all log output is also sent to each sink through the send function.
// In this implementation, WaitTillSent() is called immediately after the send.
// This implementation is not thread safe.
-class CERES_NO_EXPORT LogSink {
+class CERES_EXPORT LogSink {
public:
virtual ~LogSink() = default;
virtual void send(LogSeverity severity,
@@ -146,7 +146,7 @@
};
// Global set of log sinks. The actual object is defined in logging.cc.
-extern CERES_NO_EXPORT std::set<LogSink*> log_sinks_global;
+extern CERES_EXPORT std::set<LogSink*> log_sinks_global;
inline void InitGoogleLogging(const char* /* argv */) {
// Do nothing; this is ignored.
@@ -169,7 +169,7 @@
// defined, output is directed to std::cerr. This class should not
// be directly instantiated in code, rather it should be invoked through the
// use of the log macros LG, LOG, or VLOG.
-class CERES_NO_EXPORT MessageLogger {
+class CERES_EXPORT MessageLogger {
public:
MessageLogger(const char* file, int line, const char* tag, int severity)
: file_(file), line_(line), tag_(tag), severity_(severity) {
@@ -288,7 +288,7 @@
// This class is used to explicitly ignore values in the conditional
// logging macros. This avoids compiler warnings like "value computed
// is not used" and "statement has no effect".
-class CERES_NO_EXPORT LoggerVoidify {
+class CERES_EXPORT LoggerVoidify {
public:
// This has to be an operator with a precedence lower than << but
// higher than ?:
diff --git a/internal/ceres/rotation_test.cc b/internal/ceres/rotation_test.cc
index d97e385..d6de650 100644
--- a/internal/ceres/rotation_test.cc
+++ b/internal/ceres/rotation_test.cc
@@ -88,27 +88,29 @@
MATCHER_P(IsNearQuaternion, expected, "") {
// Quaternions are equivalent upto a sign change. So we will compare
// both signs before declaring failure.
- bool near = true;
+ bool is_near = true;
+ // NOTE: near (and far) can be defined as macros on the Windows platform (for
+ // ancient pascal calling convention). Do not use these identifiers.
for (int i = 0; i < 4; i++) {
if (fabs(arg[i] - expected[i]) > kTolerance) {
- near = false;
+ is_near = false;
break;
}
}
- if (near) {
+ if (is_near) {
return true;
}
- near = true;
+ is_near = true;
for (int i = 0; i < 4; i++) {
if (fabs(arg[i] + expected[i]) > kTolerance) {
- near = false;
+ is_near = false;
break;
}
}
- if (near) {
+ if (is_near) {
return true;
}
