Skip to content

Fix MSVC warnings (C4244, C4267, C4305, C4838) - #397

Open
MajorMurphy wants to merge 4 commits into
google:mainfrom
murphyindustries:fix-msvc-warnings
Open

Fix MSVC warnings (C4244, C4267, C4305, C4838)#397
MajorMurphy wants to merge 4 commits into
google:mainfrom
murphyindustries:fix-msvc-warnings

Conversation

@MajorMurphy

@MajorMurphy MajorMurphy commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Goal

CMakeLists.txt mentioned a TODO task of fixing several MSVC warnings

# Disable specific warnings
# TODO: None of these should be disabled, but for now, for a warning-free msvc build these are
# added. fix the warnings and remove these filters
add_compile_options(/wd4244) # conversion from 'type1' to 'type2', possible loss of data
add_compile_options(/wd4267) # conversion from 'size_t' to 'type' possible loss of data
add_compile_options(/wd4305) # truncation from 'double' to 'float'
add_compile_options(/wd4838) # conversion from 'type1' to 'type2' requires a narrowing conversion
add_compile_options(/wd26812) # Prefer enum class over enum

Solution

Upon removing the suppressors, warnings were not visible until explicitly setting warning level /W3. MSVC defaults to /W1, which is not high enough to display these warning messages. Setting it to /W3 makes it consistent with the -Wall -Wextra for non-MSVC builds

target_compile_options(${UHDR_CORE_LIB_NAME} PRIVATE /W3)

All occurrences of C4244, C4267, C4305, C4838 were resolved.
Edited files:

  • gainmapmath.h
  • gainmapmath.cpp
  • icc.cpp
  • jpegdecoderhelper.cpp
  • jpegencoderhelper.cpp
  • jpegr.cpp
  • multipictureformat.cpp

C26812 was not observed, even when building with /analyze, so that warning suppression was also removed.

Further warning messages do occur at W4 level, which could be addressed in a future pull request.

Tested

Visual Studio 2026 :

  • Successfully compiled and passed all unit tests
  • Tested basic decode/encode functionality
  • SDK 10.0.26100.0, targeting Win 10.0.26200

Ubuntu:

  • Successfully compiled and passed all unit tests
  • Clang 18.1.3

MajorMurphy and others added 4 commits August 20, 2026 12:11
- icc.cpp: use float literals in Matrix3x3 aggregate initializer (C4838)
- icc.cpp: explicit cast for strlen result assigned to uint32_t (C4267)
- jpegdecoderhelper.cpp: explicit cast for std::ceil result assigned to
  unsigned int (C4244)
- jpegencoderhelper.cpp: explicit casts for std::ceil and strlen results
  assigned to unsigned int (C4244, C4267)
- CMakeLists.txt: remove /wd4244, /wd4267, /wd4305, /wd4838 suppressions
including: C4244, C4267, C4305, C4838
The v2.0.0 container work added call sites that narrow size_t to the int
lengths libheif's API takes, and none of them had been compiled at /W3
before. Adds the missing casts so the warning-level change in the previous
commit leaves the core target warning-free:

  heifultrahdr.cpp, avifultrahdr.cpp  heif_context_add_exif_metadata and
                                      heif_context_encode_gain_map_image
  ultrahdr_api.cpp                    heif_check_filetype (x2),
                                      heif_image_handle_get_gain_map_metadata_size,
                                      and the exif_size argument of
                                      parseGainMapMetadata
  jpegr.cpp                           the same parseGainMapMetadata argument
  jpegrutils.cpp                      pow() returns double; altHeadroom is float

Wrapped by hand rather than with clang-format: these files are already 55 to
92 lines away from the repo's own .clang-format, so running it would bury the
change in unrelated reformatting. Every touched line is within the 100 column
limit.
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.

1 participant