Raise MSVC warning level from /W3 to /W4

kbenzie · kbenzie · commit 568779ec4cbb · 2024-09-24T14:24:23.000+01:00
This patch increases the warning level when using the MSVC compiler from
`/W3` to `/W4` and fixes the issues found. Four warnings introduced by
`/W4` are disabled, all related to variable name shadowing, as they
overly prescriptive to valid code.
diff --git a/cmake/helpers.cmake b/cmake/helpers.cmake
@@ -82,7 +82,11 @@ function(add_ur_target_compile_options name)
     elseif(MSVC)
         target_compile_options(${name} PRIVATE
             $<$<CXX_COMPILER_ID:MSVC>:/MP>  # clang-cl.exe does not support /MP
-            /W3
+            /W4
+            /wd4456  # Disable: declaration of 'identifier' hides previous local declaration
+            /wd4457  # Disable: declaration of 'identifier' hides function parameter
+            /wd4458  # Disable: declaration of 'identifier' hides class member
+            /wd4459  # Disable: declaration of 'identifier' hides global declaration
             /MD$<$<CONFIG:Debug>:d>
             /GS
             /DWIN32_LEAN_AND_MEAN
diff --git a/examples/collector/collector.cpp b/examples/collector/collector.cpp
@@ -25,7 +25,14 @@
 #include <string_view>
 
 #include "ur_api.h"
+
+#ifdef _MSC_VER
+#pragma warning(disable : 4245)
+#endif
 #include "xpti/xpti_trace_framework.h"
+#ifdef _MSC_VER
+#pragma warning(default : 4245)
+#endif
 
 constexpr uint16_t TRACE_FN_BEGIN =
     static_cast<uint16_t>(xpti::trace_point_type_t::function_with_args_begin);
diff --git a/include/ur_api.h b/include/ur_api.h
@@ -422,7 +422,7 @@ typedef struct ur_physical_mem_handle_t_ *ur_physical_mem_handle_t;
 ///////////////////////////////////////////////////////////////////////////////
 #ifndef UR_BIT
 /// @brief Generic macro for enumerator bit masks
-#define UR_BIT(_i) (1 << _i)
+#define UR_BIT(_i) (1U << _i)
 #endif // UR_BIT
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/scripts/core/common.yml b/scripts/core/common.yml
@@ -134,7 +134,7 @@ name: "$x_physical_mem_handle_t"
 type: macro
 desc: "Generic macro for enumerator bit masks"
 name: "$X_BIT( _i )"
-value: "( 1 << _i )"
+value: "( 1U << _i )"
 --- #--------------------------------------------------------------------------
 type: enum
 desc: "Defines Return/Error codes"
diff --git a/source/adapters/level_zero/CMakeLists.txt b/source/adapters/level_zero/CMakeLists.txt
@@ -65,7 +65,7 @@ if (NOT DEFINED LEVEL_ZERO_LIBRARY OR NOT DEFINED LEVEL_ZERO_INCLUDE_DIR)
 
     target_compile_options(ze_loader PRIVATE
         $<$<IN_LIST:$<CXX_COMPILER_ID>,GNU;Clang;Intel;IntelLLVM>:-Wno-error>
-        $<$<CXX_COMPILER_ID:MSVC>:/WX- /UUNICODE>
+        $<$<CXX_COMPILER_ID:MSVC>:/UUNICODE>
     )
 
     set(LEVEL_ZERO_LIBRARY ze_loader)
@@ -256,7 +256,7 @@ if(UR_BUILD_ADAPTER_L0_V2)
 
     # TODO: fix level_zero adapter conversion warnings
     target_compile_options(ur_adapter_level_zero_v2 PRIVATE
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244>
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244 /wd4100>
     )
 
     set_target_properties(ur_adapter_level_zero_v2 PROPERTIES
diff --git a/source/adapters/level_zero/adapter.cpp b/source/adapters/level_zero/adapter.cpp
@@ -378,7 +378,5 @@ ur_result_t urAdapterGetInfo(ur_adapter_handle_t, ur_adapter_info_t PropName,
   default:
     return UR_RESULT_ERROR_INVALID_ENUMERATION;
   }
-
-  return UR_RESULT_SUCCESS;
 }
 } // namespace ur::level_zero
diff --git a/source/adapters/level_zero/context.cpp b/source/adapters/level_zero/context.cpp
@@ -756,7 +756,7 @@ ur_result_t ur_context_handle_t_::getAvailableCommandList(
         // queue's map to hold the fence and other associated command
         // list information.
         auto &QGroup = Queue->getQueueGroup(UseCopyEngine);
-        uint32_t QueueGroupOrdinal;
+        uint32_t QueueGroupOrdinal = 0;
         auto &ZeCommandQueue = ForcedCmdQueue
                                    ? *ForcedCmdQueue
                                    : QGroup.getZeQueue(&QueueGroupOrdinal);
diff --git a/source/adapters/level_zero/device.cpp b/source/adapters/level_zero/device.cpp
@@ -1110,8 +1110,6 @@ ur_result_t urDeviceGetInfo(
                   logger::toHex(ParamName));
     return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
   }
-
-  return UR_RESULT_SUCCESS;
 }
 
 bool CopyEngineRequested(const ur_device_handle_t &Device) {
diff --git a/source/adapters/level_zero/event.cpp b/source/adapters/level_zero/event.cpp
@@ -479,8 +479,6 @@ ur_result_t urEventGetInfo(
         PropName, logger::toHex(PropName));
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
-
-  return UR_RESULT_SUCCESS;
 }
 
 ur_result_t urEventGetProfilingInfo(
diff --git a/source/adapters/level_zero/platform.cpp b/source/adapters/level_zero/platform.cpp
@@ -99,8 +99,6 @@ ur_result_t urPlatformGetInfo(
     logger::debug("urPlatformGetInfo: unrecognized ParamName");
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
-
-  return UR_RESULT_SUCCESS;
 }
 
 ur_result_t urPlatformGetApiVersion(
diff --git a/source/adapters/level_zero/queue.cpp b/source/adapters/level_zero/queue.cpp
@@ -2263,7 +2263,7 @@ ur_result_t ur_queue_handle_t_::createCommandList(
   ZeStruct<ze_fence_desc_t> ZeFenceDesc;
   ze_command_list_handle_t ZeCommandList;
 
-  uint32_t QueueGroupOrdinal;
+  uint32_t QueueGroupOrdinal = 0;
   auto &QGroup = getQueueGroup(UseCopyEngine);
   auto &ZeCommandQueue =
       ForcedCmdQueue ? *ForcedCmdQueue : QGroup.getZeQueue(&QueueGroupOrdinal);
diff --git a/source/adapters/level_zero/usm.cpp b/source/adapters/level_zero/usm.cpp
@@ -667,7 +667,6 @@ ur_result_t urUSMGetMemAllocInfo(
     logger::error("urUSMGetMemAllocInfo: unsupported ParamName");
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
-  return UR_RESULT_SUCCESS;
 }
 
 ur_result_t urUSMPoolCreate(
diff --git a/source/adapters/level_zero/virtual_mem.cpp b/source/adapters/level_zero/virtual_mem.cpp
@@ -38,7 +38,6 @@ ur_result_t urVirtualMemGranularityGetInfo(
                   propName, propName);
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
-  return UR_RESULT_SUCCESS;
 }
 
 ur_result_t urVirtualMemReserve(ur_context_handle_t hContext,
@@ -119,7 +118,5 @@ ur_result_t urVirtualMemGetInfo(ur_context_handle_t hContext,
                   propName, propName);
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
-
-  return UR_RESULT_SUCCESS;
 }
 } // namespace ur::level_zero
diff --git a/source/adapters/opencl/adapter.cpp b/source/adapters/opencl/adapter.cpp
@@ -98,6 +98,4 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetInfo(ur_adapter_handle_t,
   default:
     return UR_RESULT_ERROR_INVALID_ENUMERATION;
   }
-
-  return UR_RESULT_SUCCESS;
 }
diff --git a/source/adapters/opencl/event.cpp b/source/adapters/opencl/event.cpp
@@ -10,6 +10,7 @@
 
 #include "common.hpp"
 
+#include <limits>
 #include <mutex>
 #include <set>
 #include <unordered_map>
@@ -32,8 +33,7 @@ cl_event_info convertUREventInfoToCL(const ur_event_info_t PropName) {
     return CL_EVENT_REFERENCE_COUNT;
     break;
   default:
-    return -1;
-    break;
+    return std::numeric_limits<cl_event_info>::max();
   }
 }
 
@@ -51,7 +51,7 @@ convertURProfilingInfoToCL(const ur_profiling_info_t PropName) {
   case UR_PROFILING_INFO_COMMAND_END:
     return CL_PROFILING_COMMAND_END;
   default:
-    return -1;
+    return std::numeric_limits<cl_profiling_info>::max();
   }
 }
 
diff --git a/source/adapters/opencl/memory.cpp b/source/adapters/opencl/memory.cpp
@@ -9,6 +9,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "common.hpp"
+#include <limits>
 
 cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
   cl_image_format CLImageFormat;
@@ -59,7 +60,8 @@ cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
     CLImageFormat.image_channel_order = CL_sRGBA;
     break;
   default:
-    CLImageFormat.image_channel_order = -1;
+    CLImageFormat.image_channel_order =
+        std::numeric_limits<cl_channel_order>::max();
     break;
   }
 
@@ -110,7 +112,8 @@ cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
     CLImageFormat.image_channel_data_type = CL_FLOAT;
     break;
   default:
-    CLImageFormat.image_channel_data_type = -1;
+    CLImageFormat.image_channel_data_type =
+        std::numeric_limits<cl_channel_type>::max();
     break;
   }
 
@@ -139,7 +142,7 @@ cl_image_desc mapURImageDescToCL(const ur_image_desc_t *PImageDesc) {
     CLImageDesc.image_type = CL_MEM_OBJECT_IMAGE1D_ARRAY;
     break;
   default:
-    CLImageDesc.image_type = -1;
+    CLImageDesc.image_type = std::numeric_limits<cl_mem_object_type>::max();
     break;
   }
 
diff --git a/source/adapters/opencl/queue.cpp b/source/adapters/opencl/queue.cpp
@@ -8,9 +8,9 @@
 
 #include "common.hpp"
 #include "platform.hpp"
+#include <limits>
 
 cl_command_queue_info mapURQueueInfoToCL(const ur_queue_info_t PropName) {
-
   switch (PropName) {
   case UR_QUEUE_INFO_CONTEXT:
     return CL_QUEUE_CONTEXT;
@@ -25,7 +25,7 @@ cl_command_queue_info mapURQueueInfoToCL(const ur_queue_info_t PropName) {
   case UR_QUEUE_INFO_SIZE:
     return CL_QUEUE_SIZE;
   default:
-    return -1;
+    return std::numeric_limits<cl_command_queue_info>::max();
   }
 }
 
diff --git a/source/adapters/opencl/usm_p2p.cpp b/source/adapters/opencl/usm_p2p.cpp
@@ -8,22 +8,20 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "common.hpp"
+#include "logger/ur_logger.hpp"
 
 UR_APIEXPORT ur_result_t UR_APICALL
 urUsmP2PEnablePeerAccessExp([[maybe_unused]] ur_device_handle_t commandDevice,
                             [[maybe_unused]] ur_device_handle_t peerDevice) {
-
-  cl_adapter::die(
+  logger::warning(
       "Experimental P2P feature is not implemented for OpenCL adapter.");
   return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
 }
 
 UR_APIEXPORT ur_result_t UR_APICALL
 urUsmP2PDisablePeerAccessExp([[maybe_unused]] ur_device_handle_t commandDevice,
                              [[maybe_unused]] ur_device_handle_t peerDevice) {
-
-  cl_adapter::die(
+  logger::warning(
       "Experimental P2P feature is not implemented for OpenCL adapter.");
   return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
 }
@@ -34,8 +32,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
     [[maybe_unused]] ur_exp_peer_info_t propName,
     [[maybe_unused]] size_t propSize, [[maybe_unused]] void *pPropValue,
     [[maybe_unused]] size_t *pPropSizeRet) {
-
-  cl_adapter::die(
+  logger::warning(
       "Experimental P2P feature is not implemented for OpenCL adapter.");
   return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
 }
diff --git a/source/common/logger/ur_logger.hpp b/source/common/logger/ur_logger.hpp
@@ -116,8 +116,9 @@ template <typename T> inline std::string toHex(T t) {
 inline Logger create_logger(std::string logger_name, bool skip_prefix,
                             bool skip_linebreak,
                             logger::Level default_log_level) {
-    std::transform(logger_name.begin(), logger_name.end(), logger_name.begin(),
-                   ::toupper);
+    std::transform(
+        logger_name.begin(), logger_name.end(), logger_name.begin(),
+        [](char c) -> char { return static_cast<char>(::toupper(c)); });
     std::stringstream env_var_name;
     const auto default_flush_level = logger::Level::ERR;
     const std::string default_output = "stderr";
diff --git a/source/common/ur_util.cpp b/source/common/ur_util.cpp
@@ -15,7 +15,7 @@
 #include <windows.h>
 int ur_getpid(void) { return static_cast<int>(GetCurrentProcessId()); }
 
-int ur_close_fd(int fd) { return -1; }
+int ur_close_fd(int fd [[maybe_unused]]) { return -1; }
 
 int ur_duplicate_fd(int pid, int fd_in) {
     // TODO: find another way to obtain a duplicate of another process's file descriptor
diff --git a/source/common/ur_util.hpp b/source/common/ur_util.hpp
@@ -98,8 +98,9 @@ std::optional<std::string> ur_getenv(const char *name);
 
 inline bool getenv_tobool(const char *name, bool def = false) {
     if (auto env = ur_getenv(name); env) {
-        std::transform(env->begin(), env->end(), env->begin(),
-                       [](unsigned char c) { return std::tolower(c); });
+        std::transform(env->begin(), env->end(), env->begin(), [](char c) {
+            return static_cast<char>(std::tolower(c));
+        });
         auto true_str = {"y", "yes", "t", "true", "1"};
         return std::find(true_str.begin(), true_str.end(), *env) !=
                true_str.end();
diff --git a/source/loader/layers/tracing/ur_tracing_layer.cpp b/source/loader/layers/tracing/ur_tracing_layer.cpp
@@ -12,8 +12,16 @@
 #include "ur_tracing_layer.hpp"
 #include "ur_api.h"
 #include "ur_util.hpp"
+
+#ifdef _MSC_VER
+#pragma warning(disable : 4245)
+#endif
 #include "xpti/xpti_data_types.h"
 #include "xpti/xpti_trace_framework.h"
+#ifdef _MSC_VER
+#pragma warning(default : 4245)
+#endif
+
 #include <atomic>
 #include <optional>
 #include <sstream>
diff --git a/source/loader/ur_adapter_registry.hpp b/source/loader/ur_adapter_registry.hpp
@@ -225,12 +225,13 @@ class AdapterRegistry {
             }
 
             // case-insensitive comparison by converting both tolower
-            std::transform(platformBackendName.begin(),
-                           platformBackendName.end(),
-                           platformBackendName.begin(),
-                           [](unsigned char c) { return std::tolower(c); });
-            std::transform(backend.begin(), backend.end(), backend.begin(),
-                           [](unsigned char c) { return std::tolower(c); });
+            std::transform(
+                platformBackendName.begin(), platformBackendName.end(),
+                platformBackendName.begin(),
+                [](char c) { return static_cast<char>(std::tolower(c)); });
+            std::transform(
+                backend.begin(), backend.end(), backend.begin(),
+                [](char c) { return static_cast<char>(std::tolower(c)); });
             std::size_t nameFound = platformBackendName.find(backend);
 
             bool backendFound = nameFound != std::string::npos;
diff --git a/source/loader/ur_lib.cpp b/source/loader/ur_lib.cpp
@@ -17,6 +17,7 @@
 #define NOMINMAX
 #include "ur_api.h"
 #include "ur_ldrddi.hpp"
+#include <limits>
 #endif // !NOMINMAX
 
 #include "logger/ur_logger.hpp"
@@ -413,7 +414,7 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
 
     using DeviceIdType = unsigned long;
     constexpr DeviceIdType DeviceIdTypeALL =
-        -1; // ULONG_MAX but without #include <climits>
+        std::numeric_limits<DeviceIdType>::max();
 
     struct DeviceSpec {
         DevicePartLevel level;
@@ -427,8 +428,9 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
     auto getRootHardwareType =
         [](const std::string &input) -> DeviceHardwareType {
         std::string lowerInput(input);
-        std::transform(lowerInput.cbegin(), lowerInput.cend(),
-                       lowerInput.begin(), ::tolower);
+        std::transform(
+            lowerInput.cbegin(), lowerInput.cend(), lowerInput.begin(),
+            [](char c) { return static_cast<char>(std::tolower(c)); });
         if (lowerInput == "cpu") {
             return ::UR_DEVICE_TYPE_CPU;
         }
@@ -483,9 +485,8 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
                         platformBackendName.cend(), backend.cbegin(),
                         backend.cend(), [](const auto &a, const auto &b) {
                             // case-insensitive comparison by converting both tolower
-                            return std::tolower(
-                                       static_cast<unsigned char>(a)) ==
-                                   std::tolower(static_cast<unsigned char>(b));
+                            return std::tolower(static_cast<char>(a)) ==
+                                   std::tolower(static_cast<char>(b));
                         })) {
             // irrelevant term for current request: different backend -- silently ignore
             logger::error("unrecognised backend '{}'", backend);
diff --git a/source/loader/windows/adapter_search.cpp b/source/loader/windows/adapter_search.cpp
@@ -40,7 +40,8 @@ std::optional<fs::path> getLoaderLibPath() {
     return std::nullopt;
 }
 
-std::optional<fs::path> getAdapterNameAsPath(std::string adapterName) {
+std::optional<fs::path> getAdapterNameAsPath(std::string adapterName
+                                             [[maybe_unused]]) {
     return std::nullopt;
 }
 
diff --git a/test/conformance/device/urDevicePartition.cpp b/test/conformance/device/urDevicePartition.cpp
@@ -138,7 +138,7 @@ TEST_F(urDevicePartitionTest, PartitionByCounts) {
             uint32_t sum = 0;
             for (auto sub_device : sub_devices) {
                 ASSERT_NE(sub_device, nullptr);
-                uint32_t n_cu_in_sub_device;
+                uint32_t n_cu_in_sub_device = 0;
                 ASSERT_NO_FATAL_FAILURE(
                     getNumberComputeUnits(sub_device, n_cu_in_sub_device));
                 sum += n_cu_in_sub_device;
diff --git a/test/conformance/exp_command_buffer/buffer_fill_kernel_update.cpp b/test/conformance/exp_command_buffer/buffer_fill_kernel_update.cpp
diff --git a/test/conformance/integration/QueueBuffer.cpp b/test/conformance/integration/QueueBuffer.cpp
diff --git a/test/conformance/testing/include/uur/utils.h b/test/conformance/testing/include/uur/utils.h
diff --git a/test/conformance/usm/urUSMDeviceAlloc.cpp b/test/conformance/usm/urUSMDeviceAlloc.cpp
diff --git a/test/conformance/usm/urUSMHostAlloc.cpp b/test/conformance/usm/urUSMHostAlloc.cpp
diff --git a/test/conformance/usm/urUSMSharedAlloc.cpp b/test/conformance/usm/urUSMSharedAlloc.cpp
diff --git a/test/layers/tracing/test_collector.cpp b/test/layers/tracing/test_collector.cpp
diff --git a/tools/urinfo/utils.hpp b/tools/urinfo/utils.hpp
diff --git a/tools/urtrace/collector.cpp b/tools/urtrace/collector.cpp

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ if (NOT DEFINED LEVEL_ZERO_LIBRARY OR NOT DEFINED LEVEL_ZERO_INCLUDE_DIR)`
`65`	`65`
`66`	`66`	`target_compile_options(ze_loader PRIVATE`
`67`	`67`	`$<$<IN_LIST:$<CXX_COMPILER_ID>,GNU;Clang;Intel;IntelLLVM>:-Wno-error>`
`68`		`- $<$<CXX_COMPILER_ID:MSVC>:/WX- /UUNICODE>`
	`68`	`+ $<$<CXX_COMPILER_ID:MSVC>:/UUNICODE>`
`69`	`69`	`)`
`70`	`70`
`71`	`71`	`set(LEVEL_ZERO_LIBRARY ze_loader)`
`@@ -256,7 +256,7 @@ if(UR_BUILD_ADAPTER_L0_V2)`
`256`	`256`
`257`	`257`	`# TODO: fix level_zero adapter conversion warnings`
`258`	`258`	`target_compile_options(ur_adapter_level_zero_v2 PRIVATE`
`259`		`- $<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244>`
	`259`	`+ $<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244 /wd4100>`
`260`	`260`	`)`
`261`	`261`
`262`	`262`	`set_target_properties(ur_adapter_level_zero_v2 PROPERTIES`
Original file line number	Diff line number	Diff line change
`@@ -378,7 +378,5 @@ ur_result_t urAdapterGetInfo(ur_adapter_handle_t, ur_adapter_info_t PropName,`
`378`	`378`	`default:`
`379`	`379`	`return UR_RESULT_ERROR_INVALID_ENUMERATION;`
`380`	`380`	`}`
`381`		`-`
`382`		`- return UR_RESULT_SUCCESS;`
`383`	`381`	`}`
`384`	`382`	`} // namespace ur::level_zero`
Original file line number	Diff line number	Diff line change
`@@ -1110,8 +1110,6 @@ ur_result_t urDeviceGetInfo(`
`1110`	`1110`	`logger::toHex(ParamName));`
`1111`	`1111`	`return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;`
`1112`	`1112`	`}`
`1113`		`-`
`1114`		`- return UR_RESULT_SUCCESS;`
`1115`	`1113`	`}`
`1116`	`1114`
`1117`	`1115`	`bool CopyEngineRequested(const ur_device_handle_t &Device) {`
Original file line number	Diff line number	Diff line change
`@@ -479,8 +479,6 @@ ur_result_t urEventGetInfo(`
`479`	`479`	`PropName, logger::toHex(PropName));`
`480`	`480`	`return UR_RESULT_ERROR_INVALID_VALUE;`
`481`	`481`	`}`
`482`		`-`
`483`		`- return UR_RESULT_SUCCESS;`
`484`	`482`	`}`
`485`	`483`
`486`	`484`	`ur_result_t urEventGetProfilingInfo(`
Original file line number	Diff line number	Diff line change
`@@ -99,8 +99,6 @@ ur_result_t urPlatformGetInfo(`
`99`	`99`	`logger::debug("urPlatformGetInfo: unrecognized ParamName");`
`100`	`100`	`return UR_RESULT_ERROR_INVALID_VALUE;`
`101`	`101`	`}`
`102`		`-`
`103`		`- return UR_RESULT_SUCCESS;`
`104`	`102`	`}`
`105`	`103`
`106`	`104`	`ur_result_t urPlatformGetApiVersion(`