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
This commit is contained in:
Sergiu Deitsch
2022-02-17 00:56:27 +01:00
parent c8658c8992
commit f0851667be
16 changed files with 156 additions and 63 deletions
+11 -1
View File
@@ -444,7 +444,8 @@ endif (NOT CUSTOM_BLAS)
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 @@ add_compile_options(
$<$<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
+3 -3
View File
@@ -191,7 +191,7 @@ struct Grid1D {
}
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 @@ struct Grid2D {
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;
+4 -4
View File
@@ -82,11 +82,11 @@ void ComputeHouseholderVector(const XVectorType& x,
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)));
}
+4 -4
View File
@@ -121,7 +121,7 @@ struct NumericDiff {
// 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 @@ struct NumericDiff {
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 @@ struct NumericDiff {
// 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 @@ struct NumericDiff {
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))
+22
View File
@@ -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_
+49 -12
View File
@@ -443,15 +443,9 @@ using std::fmin;
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::sqrt;
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 @@ struct numeric_limits<ceres::Jet<T, N>> {
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 @@ struct numeric_limits<ceres::Jet<T, N>> {
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 @@ struct NumTraits<ceres::Jet<T, N>> {
};
};
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
+4 -1
View File
@@ -285,6 +285,9 @@ class CERES_EXPORT ProductManifold final : public Manifold {
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 @@ class CERES_EXPORT ProductManifold final : public Manifold {
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();
+2 -2
View File
@@ -250,7 +250,7 @@ class TinySolver {
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 @@ class TinySolver {
}
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 {
+11 -10
View File
@@ -287,7 +287,7 @@ if(BUILD_SHARED_LIBS)
# 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 @@ install(TARGETS ceres
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 @@ if (BUILD_TESTING AND GFLAGS)
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()
@@ -9,9 +9,3 @@ endif()
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})
+15 -9
View File
@@ -97,19 +97,25 @@ TEST_P(DenseCholeskyTest, FactorAndSolve) {
}
}
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
+4
View File
@@ -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"
@@ -148,7 +148,7 @@ TEST(LevenbergMarquardtStrategy, CorrectDiagonalToLinearSolver) {
// 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,
+14
View File
@@ -312,6 +312,20 @@ bool SubsetManifold::MinusJacobian(const double* x,
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_; }
+4 -4
View File
@@ -132,7 +132,7 @@ const int FATAL = ::FATAL;
// 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 @@ class CERES_NO_EXPORT LogSink {
};
// 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 @@ inline void RemoveLogSink(LogSink* sink) { log_sinks_global.erase(sink); }
// 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 @@ class CERES_NO_EXPORT MessageLogger {
// 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 ?:
+8 -6
View File
@@ -88,27 +88,29 @@ MATCHER(IsNormalizedQuaternion, "") {
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;
}