Fix more compilation warnings.

Bug: chromium:1064305
Change-Id: I3aac24f8179d2c9e5206dd4542ea2506f26755e9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19301
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@google.com>

generator/templates/dawn_wire/WireCmd.cpp

@@ -414,12 +414,12 @@ namespace dawn_wire {
 
     // Macro to simplify error handling, similar to DAWN_TRY but for DeserializeResult.
 #define DESERIALIZE_TRY(EXPR) \
-    { \
+    do { \
         DeserializeResult exprResult = EXPR; \
         if (exprResult != DeserializeResult::Success) { \
             return exprResult; \
         } \
-    }
+    } while (0)
 
     ObjectHandle::ObjectHandle() = default;
     ObjectHandle::ObjectHandle(ObjectId id, ObjectSerial serial) : id(id), serial(serial) {}

src/common/BUILD.gn

@@ -95,6 +95,12 @@ config("dawn_internal") {
       "-Wshadow-field",
       "-Wmissing-field-initializers",
       "-Wcstring-format-directive",
+      "-Wtautological-unsigned-zero-compare",
+      "-Wreturn-std-move-in-c++11",
+
+      # Turn on the following flag after removing the empty statement in
+      # third_party/glm/glm/simd/common.h:106
+      # "-Wextra-semi-stmt",
     ]
   }
 }
@@ -143,13 +149,9 @@ if (is_win || is_linux || is_mac || is_fuchsia || is_android) {
     ]
 
     public_configs = [ ":dawn_internal" ]
-    deps = [
+    deps = [ "${dawn_root}/src/dawn:dawn_headers" ]
-      "${dawn_root}/src/dawn:dawn_headers",
-    ]
     if (dawn_enable_vulkan) {
-      public_deps = [
+      public_deps = [ "${dawn_root}/third_party/khronos:vulkan_headers" ]
-        "${dawn_root}/third_party/khronos:vulkan_headers",
-      ]
     }
     if (is_android) {
       libs = [ "log" ]

src/dawn_native/BuddyMemoryAllocator.cpp

@@ -42,7 +42,7 @@ namespace dawn_native {
         ResourceMemoryAllocation invalidAllocation = ResourceMemoryAllocation{};
 
         if (allocationSize == 0) {
-            return invalidAllocation;
+            return std::move(invalidAllocation);
         }
 
         // Round allocation size to nearest power-of-two.
@@ -50,13 +50,13 @@ namespace dawn_native {
 
         // Allocation cannot exceed the memory size.
         if (allocationSize > mMemoryBlockSize) {
-            return invalidAllocation;
+            return std::move(invalidAllocation);
         }
 
         // Attempt to sub-allocate a block of the requested size.
         const uint64_t blockOffset = mBuddyBlockAllocator.Allocate(allocationSize, alignment);
         if (blockOffset == BuddyAllocator::kInvalidOffset) {
-            return invalidAllocation;
+            return std::move(invalidAllocation);
         }
 
         const uint64_t memoryIndex = GetMemoryIndex(blockOffset);

src/dawn_native/ShaderModule.cpp

@@ -700,7 +700,7 @@ namespace dawn_native {
                     SpirvCrossBaseTypeToFormatType(shaderFragmentOutputBaseType);
                 if (formatType == Format::Type::Other) {
                     return DAWN_VALIDATION_ERROR("Unexpected Fragment output type");
-                };
+                }
                 mFragmentOutputFormatBaseTypes[location] = formatType;
             }
         }

src/dawn_native/d3d12/BackendD3D12.cpp

@@ -65,7 +65,7 @@ namespace dawn_native { namespace d3d12 {
             }
 
             ASSERT(factory != nullptr);
-            return factory;
+            return std::move(factory);
         }
 
     }  // anonymous namespace

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -395,7 +395,7 @@ namespace dawn_native { namespace d3d12 {
         DAWN_TRY(CheckHRESULT(d3d11Texture.As(&dxgiKeyedMutex),
                               "D3D12 QueryInterface ID3D11Texture2D to IDXGIKeyedMutex"));
 
-        return dxgiKeyedMutex;
+        return std::move(dxgiKeyedMutex);
     }
 
     void Device::ReleaseKeyedMutexForTexture(ComPtr<IDXGIKeyedMutex> dxgiKeyedMutex) {

src/dawn_native/d3d12/DeviceD3D12.h

@@ -39,10 +39,10 @@ namespace dawn_native { namespace d3d12 {
     class StagingDescriptorAllocator;
 
 #define ASSERT_SUCCESS(hr)            \
-    {                                 \
+    do {                              \
         HRESULT succeeded = hr;       \
         ASSERT(SUCCEEDED(succeeded)); \
-    }
+    } while (0)
 
     // Definition of backend types
     class Device : public DeviceBase {

src/dawn_native/d3d12/HeapAllocatorD3D12.cpp

@@ -57,7 +57,7 @@ namespace dawn_native { namespace d3d12 {
         // Calling CreateHeap implicitly calls MakeResident on the new heap. We must track this to
         // avoid calling MakeResident a second time.
         mDevice->GetResidencyManager()->TrackResidentAllocation(ToBackend(heapBase.get()));
-        return heapBase;
+        return std::move(heapBase);
     }
 
     void HeapAllocator::DeallocateResourceHeap(std::unique_ptr<ResourceHeapBase> heap) {

src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp

@@ -162,7 +162,7 @@ namespace dawn_native { namespace d3d12 {
         DAWN_TRY_ASSIGN(subAllocation,
                         CreatePlacedResource(heapType, resourceDescriptor, initialUsage));
         if (subAllocation.GetInfo().mMethod != AllocationMethod::kInvalid) {
-            return subAllocation;
+            return std::move(subAllocation);
         }
 
         // If sub-allocation fails, fall-back to direct allocation (committed resource).
@@ -170,7 +170,7 @@ namespace dawn_native { namespace d3d12 {
         DAWN_TRY_ASSIGN(directAllocation,
                         CreateCommittedResource(heapType, resourceDescriptor, initialUsage));
 
-        return directAllocation;
+        return std::move(directAllocation);
     }
 
     void ResourceAllocatorManager::Tick(Serial completedSerial) {

src/dawn_native/d3d12/ShaderModuleD3D12.cpp

@@ -122,7 +122,7 @@ namespace dawn_native { namespace d3d12 {
             std::string result_string;
             DAWN_TRY(CheckSpvcSuccess(result.GetStringOutput(&result_string),
                                       "Unable to get HLSL shader text"));
-            return result_string;
+            return std::move(result_string);
         } else {
             return compiler->compile();
         }

src/dawn_native/metal/BackendMTL.mm

@@ -183,7 +183,7 @@ namespace dawn_native { namespace metal {
             if (!instance->ConsumedError(GetDevicePCIInfo(device, &ids))) {
                 mPCIInfo.vendorId = ids.vendorId;
                 mPCIInfo.deviceId = ids.deviceId;
-            };
+            }
 
 #if defined(DAWN_PLATFORM_IOS)
             mAdapterType = wgpu::AdapterType::IntegratedGPU;

src/dawn_native/vulkan/ResourceMemoryAllocatorVk.cpp

@@ -120,7 +120,7 @@ namespace dawn_native { namespace vulkan {
             DAWN_TRY_ASSIGN(subAllocation,
                             mAllocatorsPerType[memoryType]->AllocateMemory(requirements));
             if (subAllocation.GetInfo().mMethod != AllocationMethod::kInvalid) {
-                return subAllocation;
+                return std::move(subAllocation);
             }
         }
 

src/dawn_native/vulkan/VulkanFunctions.cpp

@@ -19,11 +19,13 @@
 
 namespace dawn_native { namespace vulkan {
 
-#define GET_GLOBAL_PROC(name)                                                          \
+#define GET_GLOBAL_PROC(name)                                                              \
-    name = reinterpret_cast<decltype(name)>(GetInstanceProcAddr(nullptr, "vk" #name)); \
+    do {                                                                                   \
-    if (name == nullptr) {                                                             \
+        name = reinterpret_cast<decltype(name)>(GetInstanceProcAddr(nullptr, "vk" #name)); \
-        return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #name);       \
+        if (name == nullptr) {                                                             \
-    }
+            return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #name);       \
+        }                                                                                  \
+    } while (0)
 
     MaybeError VulkanFunctions::LoadGlobalProcs(const DynamicLib& vulkanLib) {
         if (!vulkanLib.GetProc(&GetInstanceProcAddr, "vkGetInstanceProcAddr")) {
@@ -41,11 +43,13 @@ namespace dawn_native { namespace vulkan {
         return {};
     }
 
-#define GET_INSTANCE_PROC_BASE(name, procName)                                              \
+#define GET_INSTANCE_PROC_BASE(name, procName)                                                  \
-    name = reinterpret_cast<decltype(name)>(GetInstanceProcAddr(instance, "vk" #procName)); \
+    do {                                                                                        \
-    if (name == nullptr) {                                                                  \
+        name = reinterpret_cast<decltype(name)>(GetInstanceProcAddr(instance, "vk" #procName)); \
-        return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #procName);        \
+        if (name == nullptr) {                                                                  \
-    }
+            return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #procName);        \
+        }                                                                                       \
+    } while (0)
 
 #define GET_INSTANCE_PROC(name) GET_INSTANCE_PROC_BASE(name, name)
 #define GET_INSTANCE_PROC_VENDOR(name, vendor) GET_INSTANCE_PROC_BASE(name, name##vendor)
@@ -144,11 +148,13 @@ namespace dawn_native { namespace vulkan {
         return {};
     }
 
-#define GET_DEVICE_PROC(name)                                                       \
+#define GET_DEVICE_PROC(name)                                                           \
-    name = reinterpret_cast<decltype(name)>(GetDeviceProcAddr(device, "vk" #name)); \
+    do {                                                                                \
-    if (name == nullptr) {                                                          \
+        name = reinterpret_cast<decltype(name)>(GetDeviceProcAddr(device, "vk" #name)); \
-        return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #name);    \
+        if (name == nullptr) {                                                          \
-    }
+            return DAWN_INTERNAL_ERROR(std::string("Couldn't get proc vk") + #name);    \
+        }                                                                               \
+    } while (0)
 
     MaybeError VulkanFunctions::LoadDeviceProcs(VkDevice device,
                                                 const VulkanDeviceInfo& deviceInfo) {

src/dawn_native/vulkan/VulkanInfo.cpp

@@ -196,7 +196,7 @@ namespace dawn_native { namespace vulkan {
 
         // TODO(cwallez@chromium:org): Each layer can expose additional extensions, query them?
 
-        return info;
+        return std::move(info);
     }
 
     ResultOrError<std::vector<VkPhysicalDevice>> GetPhysicalDevices(const Backend& backend) {
@@ -215,7 +215,7 @@ namespace dawn_native { namespace vulkan {
             vkFunctions.EnumeratePhysicalDevices(instance, &count, physicalDevices.data()),
             "vkEnumeratePhysicalDevices"));
 
-        return physicalDevices;
+        return std::move(physicalDevices);
     }
 
     ResultOrError<VulkanDeviceInfo> GatherDeviceInfo(const Adapter& adapter) {
@@ -321,7 +321,7 @@ namespace dawn_native { namespace vulkan {
 
         // TODO(cwallez@chromium.org): gather info about formats
 
-        return info;
+        return std::move(info);
     }
 
     ResultOrError<VulkanSurfaceInfo> GatherSurfaceInfo(const Adapter& adapter,
@@ -382,7 +382,7 @@ namespace dawn_native { namespace vulkan {
                                     "vkGetPhysicalDeviceSurfacePresentModesKHR"));
         }
 
-        return info;
+        return std::move(info);
     }
 
 }}  // namespace dawn_native::vulkan

src/dawn_platform/tracing/TraceEvent.h

@@ -689,13 +689,15 @@
 #define INTERNAL_TRACE_EVENT_ADD_SCOPED(platform, category, name, ...)                         \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(platform, ::dawn_platform::TraceCategory::category) \
     dawn_platform::TraceEvent::TraceEndOnScopeClose INTERNALTRACEEVENTUID(profileScope);       \
-    if (*INTERNALTRACEEVENTUID(catstatic)) {                                                   \
+    do {                                                                                       \
-        dawn_platform::TraceEvent::addTraceEvent(                                              \
+        if (*INTERNALTRACEEVENTUID(catstatic)) {                                               \
-            platform, TRACE_EVENT_PHASE_BEGIN, INTERNALTRACEEVENTUID(catstatic), name,         \
+            dawn_platform::TraceEvent::addTraceEvent(                                          \
-            dawn_platform::TraceEvent::noEventId, TRACE_EVENT_FLAG_NONE, ##__VA_ARGS__);       \
+                platform, TRACE_EVENT_PHASE_BEGIN, INTERNALTRACEEVENTUID(catstatic), name,     \
-        INTERNALTRACEEVENTUID(profileScope)                                                    \
+                dawn_platform::TraceEvent::noEventId, TRACE_EVENT_FLAG_NONE, ##__VA_ARGS__);   \
-            .initialize(platform, INTERNALTRACEEVENTUID(catstatic), name);                     \
+            INTERNALTRACEEVENTUID(profileScope)                                                \
-    }
+                .initialize(platform, INTERNALTRACEEVENTUID(catstatic), name);                 \
+        }                                                                                      \
+    } while (0)
 
 // Implementation detail: internal macro to create static category and add
 // event if the category is enabled.

src/tests/DawnTest.h

@@ -63,7 +63,7 @@
     StartExpectDeviceError();          \
     statement;                         \
     FlushWire();                       \
-    ASSERT_TRUE(EndExpectDeviceError());
+    ASSERT_TRUE(EndExpectDeviceError())
 
 struct RGBA8 {
     constexpr RGBA8() : RGBA8(0, 0, 0, 0) {
@@ -314,12 +314,14 @@ class DawnTestBase {
 };
 
 // Skip a test when the given condition is satisfied.
-#define DAWN_SKIP_TEST_IF(condition)                        \
+#define DAWN_SKIP_TEST_IF(condition)                            \
-    if (condition) {                                        \
+    do {                                                        \
-        dawn::InfoLog() << "Test skipped: " #condition "."; \
+        if (condition) {                                        \
-        GTEST_SKIP();                                       \
+            dawn::InfoLog() << "Test skipped: " #condition "."; \
-        return;                                             \
+            GTEST_SKIP();                                       \
-    }
+            return;                                             \
+        }                                                       \
+    } while (0)
 
 template <typename Params = DawnTestParam>
 class DawnTestWithParams : public DawnTestBase, public ::testing::TestWithParam<Params> {

src/tests/end2end/TextureZeroInitTests.cpp

@@ -18,15 +18,17 @@
 #include "utils/ComboRenderPipelineDescriptor.h"
 #include "utils/WGPUHelpers.h"
 
-#define EXPECT_LAZY_CLEAR(N, statement)                                                   \
+#define EXPECT_LAZY_CLEAR(N, statement)                                                       \
-    if (UsesWire()) {                                                                     \
+    do {                                                                                      \
-        statement;                                                                        \
+        if (UsesWire()) {                                                                     \
-    } else {                                                                              \
+            statement;                                                                        \
-        size_t lazyClearsBefore = dawn_native::GetLazyClearCountForTesting(device.Get()); \
+        } else {                                                                              \
-        statement;                                                                        \
+            size_t lazyClearsBefore = dawn_native::GetLazyClearCountForTesting(device.Get()); \
-        size_t lazyClearsAfter = dawn_native::GetLazyClearCountForTesting(device.Get());  \
+            statement;                                                                        \
-        EXPECT_EQ(N, lazyClearsAfter - lazyClearsBefore);                                 \
+            size_t lazyClearsAfter = dawn_native::GetLazyClearCountForTesting(device.Get());  \
-    }
+            EXPECT_EQ(N, lazyClearsAfter - lazyClearsBefore);                                 \
+        }                                                                                     \
+    } while (0)
 
 class TextureZeroInitTest : public DawnTest {
   protected:

src/tests/unittests/validation/ValidationTest.h

@@ -22,7 +22,7 @@
 #define ASSERT_DEVICE_ERROR(statement) \
     StartExpectDeviceError(); \
     statement; \
-    ASSERT_TRUE(EndExpectDeviceError());
+    ASSERT_TRUE(EndExpectDeviceError())
 
 class ValidationTest : public testing::Test {
   public:

src/tests/white_box/D3D12ResidencyTests.cpp

@@ -194,7 +194,7 @@ TEST_P(D3D12ResidencyTests, OvercommitLargeResources) {
 TEST_P(D3D12ResidencyTests, AsyncMappedBufferRead) {
     // Dawn currently only manages LOCAL_MEMORY. Mappable buffers exist in NON_LOCAL_MEMORY on
     // discrete devices.
-    DAWN_SKIP_TEST_IF(!IsUMA())
+    DAWN_SKIP_TEST_IF(!IsUMA());
 
     // Create a mappable buffer.
     wgpu::Buffer buffer = CreateBuffer(4, wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst);
@@ -239,7 +239,7 @@ TEST_P(D3D12ResidencyTests, AsyncMappedBufferRead) {
 TEST_P(D3D12ResidencyTests, AsyncMappedBufferWrite) {
     // Dawn currently only manages LOCAL_MEMORY. Mappable buffers exist in NON_LOCAL_MEMORY on
     // discrete devices.
-    DAWN_SKIP_TEST_IF(!IsUMA())
+    DAWN_SKIP_TEST_IF(!IsUMA());
 
     // Create a mappable buffer.
     wgpu::Buffer buffer = CreateBuffer(4, wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc);

src/utils/GLFWUtils.cpp

@@ -61,7 +61,7 @@ namespace utils {
             std::make_unique<wgpu::SurfaceDescriptorFromWindowsHWND>();
         desc->hwnd = glfwGetWin32Window(window);
         desc->hinstance = GetModuleHandle(nullptr);
-        return desc;
+        return std::move(desc);
     }
 #elif defined(DAWN_USE_X11)
     std::unique_ptr<wgpu::ChainedStruct> SetupWindowAndGetSurfaceDescriptorForTesting(
@@ -70,7 +70,7 @@ namespace utils {
             std::make_unique<wgpu::SurfaceDescriptorFromXlib>();
         desc->display = glfwGetX11Display();
         desc->window = glfwGetX11Window(window);
-        return desc;
+        return std::move(desc);
     }
 #elif defined(DAWN_ENABLE_BACKEND_METAL)
     // SetupWindowAndGetSurfaceDescriptorForTesting defined in GLFWUtils_metal.mm

src/utils/GLFWUtils_metal.mm

@@ -45,7 +45,7 @@ namespace utils {
             std::unique_ptr<wgpu::SurfaceDescriptorFromMetalLayer> desc =
                 std::make_unique<wgpu::SurfaceDescriptorFromMetalLayer>();
             desc->layer = [view layer];
-            return desc;
+            return std::move(desc);
         }
 
         return nullptr;