Skip to content

Fix MSVC warnings C4805, C4996, and C4756 - #4910

Merged
sarlinpe merged 1 commit into
mainfrom
fix-msvc-warnings
Oct 28, 2026
Merged

sarlinpe merged 1 commit into
mainfrom
fix-msvc-warnings

Conversation

@sarlinpe

Copy link
Copy Markdown
Member

Fixes the following warnings from the windows-2025-vs2026 Release CI job (https://github.com/colmap/colmap/actions/runs/40514164020/job/122223590165), without changing behavior:

  • C4805 ('==': unsafe mix of type 'const Lhs' and type 'const T') in pose_test.cc / generalized_pose_test.cc: the std::vector<char> inlier masks were matched with testing::Each(testing::Eq(true)). They are now compared against static_cast<char>(true), which is the same comparison (char == 1).
  • C4996 ('getenv': This function or variable may be unsafe) in loma_test.cc (and the same pattern in single_view_calibration_test.cc): the existing portable GetEnvSafe helper (getenv_s on MSVC) moves from an anonymous namespace in util/file.cc to the public util/file.h API, and the tests now use it. Added a unit test for it.
  • C4756 (overflow in constant arithmetic) at math.h(299): MSVC constant-folds static_cast<float>(1e300) in TruncateCast<double, float> even though that cast can't be reached after the range checks. The clamp-then-cast branch, previously used only for floating-to-integer, now also covers floating-to-floating conversions whose output bounds are exactly representable in the input type, so the cast never sees an out-of-range value. I compared the old and new implementations bit for bit on ±0, ±inf, NaN, the neighbors of each bound, and 2M random bit patterns for double→float, double→double, float→float, float→double, float→int32, double→int32, float→uint8 and double→int64. They match exactly.

- C4805: Compare std::vector<char> inlier masks against a char instead of
  bool in gmock Eq matchers.
- C4996: Expose GetEnvSafe in util/file.h and use it instead of
  std::getenv in tests.
- C4756: In TruncateCast, clamp floating-point values to floating-point
  output bounds before casting when they are exactly representable, so
  that the cast can never overflow after constant folding.
@sarlinpe
sarlinpe requested a review from ahojnnes October 28, 2026 00:43
@sarlinpe
sarlinpe enabled auto-merge (squash) October 28, 2026 00:44
@sarlinpe
sarlinpe merged commit c0339e9 into main Oct 28, 2026
16 checks passed
@sarlinpe
sarlinpe deleted the fix-msvc-warnings branch October 28, 2026 07:39
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 3, 2026
## Summary
- Integrate all 12 upstream commits after the shared base 9371f7b, including LoMa BF16 loading, matching-worker error propagation, keypoint shape scaling, and project-file CLI overrides.
- Preserve the fork's CUDA blocking-sync helper alongside upstream's BF16 device check and register the shared CUDA test once.

## Validation
- Resolved all merge conflicts; no conflict markers or unmerged paths remain; git diff --check passes.
- Incremental Linux CUDA build linked six focused test executables. The prepared custom Ceres closure required its existing Abseil/CUDA library directories on the linker's search path.
- CTest passed controllers/base_option_manager_test, feature_matching_utils_test, incremental_pipeline_test, option_manager_test, feature/loma_test, and util/cuda_test (six programs, one run each).
- No full suite, Python binding, end-to-end, or performance claim.

## Upstream commits

- `ea470c0c4f4a40a3bdfa33d01eb38375c5df64de` Run Windows and remaining pycolmap CI only with the full-ci label (colmap#4904)
- `c0339e95482c404b51f38ff9b86866de495984f0` Fix MSVC warnings C4805, C4996, and C4756 (colmap#4910)
- `f00d6063bb0b120cf10ee190f7f06f4a34edf432` Expose Ceres solver thresholds in incremental mapping (colmap#4892)
- `978a4467f36bbfc9ae18780f2dca253000b0d008` Keep division-camera derivatives finite on the optical axis (colmap#4888)
- `98f31541f483788ab550f5c4a4e0804e8674b9e0` Fix leak and check covariance in CovarianceWeightedCostFunction (colmap#4911)
- `a467519441171054c7aaa17dffe86fe06431f05c` Avoid stack overflow when compressing union-find chains (colmap#4906)
- `404b83db2c2cc04892e14a578ddfa73e0ac9bfb1` Fix LoMa bf16 fallback on CUDA by loading it like any other model (colmap#4924)
- `d02e6701dede22eaeee89f73784a8c79f6f289ac` Add max_num_features option to automatic_reconstructor and benchmark (colmap#4907)
- `82c3b5db78d96a7e93ca7b8b2af0634e28aa33f4` Propagate errors of feature matching workers instead of terminating (colmap#4921)
- `4afe8d1156a71a2ff33974895f221c8eb3740cb1` Rescale keypoint shapes in image coordinates (colmap#4926)
- `ce2b5ab9df3801c2b75edbba338c04ad3c8cc6af` Fix vocabulary tree builder descriptor dimensions (colmap#4923)
- `f956ce629c21a88d75a7f7b3107298d1faf3dcf3` Fix command-line overrides for project files (colmap#4902)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants