Fix readability/todo issues.

This CL adds names to the TODOs which did not have them and enables
the readability/todo lint. The names are based on the git blame for
the TODO lines.

Bug: dawn:1339
Change-Id: I25a2920bc8fa9606f5dda67a629fdef1c10f8948
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/87600
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>

src/dawn/CPPLINT.cfg

@@ -1,2 +1 @@
-filter=-readability/todo
 filter=-runtime/indentation_namespace

src/dawn/common/StackContainer.h

@@ -83,8 +83,6 @@ class StackAllocator : public std::allocator<T> {
     // For this constructor, we cannot share storage; there's
     // no guarantee that the Source buffer of Ts is large enough
     // for Us.
-    // TODO: If we were fancy pants, perhaps we could share storage
-    // iff sizeof(T) == sizeof(U).
     template <typename U, size_t other_capacity>
     StackAllocator(const StackAllocator<U, other_capacity>& other) : source_(nullptr) {
     }

src/dawn/common/SystemUtils.cpp

@@ -126,7 +126,7 @@ std::optional<std::string> GetExecutablePath() {
 }
 #elif defined(DAWN_PLATFORM_FUCHSIA)
 std::optional<std::string> GetExecutablePath() {
-    // TODO: Implement on Fuchsia
+    UNIMPLEMENTED();
     return {};
 }
 #elif defined(DAWN_PLATFORM_EMSCRIPTEN)

src/dawn/native/d3d12/RenderPassBuilderD3D12.cpp

@@ -69,19 +69,9 @@ namespace dawn::native::d3d12 {
             }
 
             // RESOLVE_MODE_AVERAGE is only valid for non-integer formats.
-            // TODO: Investigate and determine how integer format resolves should work in WebGPU.
-            switch (resolveDestination->GetFormat().GetAspectInfo(Aspect::Color).baseType) {
-                case wgpu::TextureComponentType::Sint:
-                case wgpu::TextureComponentType::Uint:
-                    resolveParameters.ResolveMode = D3D12_RESOLVE_MODE_MAX;
-                    break;
-                case wgpu::TextureComponentType::Float:
-                    resolveParameters.ResolveMode = D3D12_RESOLVE_MODE_AVERAGE;
-                    break;
-
-                case wgpu::TextureComponentType::DepthComparison:
-                    UNREACHABLE();
-            }
+            ASSERT(resolveDestination->GetFormat().GetAspectInfo(Aspect::Color).baseType ==
+                   wgpu::TextureComponentType::Float);
+            resolveParameters.ResolveMode = D3D12_RESOLVE_MODE_AVERAGE;
 
             resolveParameters.SubresourceCount = 1;
 

src/dawn/native/metal/RenderPipelineMTL.mm

@@ -327,8 +327,8 @@ namespace dawn::native::metal {
             AcquireNSRef([MTLRenderPipelineDescriptor new]);
         MTLRenderPipelineDescriptor* descriptorMTL = descriptorMTLRef.Get();
 
-        // TODO: MakeVertexDesc should be const in the future, so we don't need to call it here when
-        // vertex pulling is enabled
+        // TODO(dawn:1384): MakeVertexDesc should be const in the future, so we don't need to call
+        // it here when vertex pulling is enabled
         NSRef<MTLVertexDescriptor> vertexDesc = MakeVertexDesc();
 
         // Calling MakeVertexDesc first is important since it sets indices for packed bindings

src/dawn/native/metal/TextureMTL.mm

@@ -317,7 +317,7 @@ namespace dawn::native::metal {
                 if (@available(macOS 10.12, iOS 13.0, *)) {
                     return MTLPixelFormatDepth16Unorm;
                 } else {
-                    // TODO (dawn:1181): Allow non-conformant implementation on macOS 10.11
+                    // TODO(dawn:1181): Allow non-conformant implementation on macOS 10.11
                     UNREACHABLE();
                 }
             case wgpu::TextureFormat::Stencil8:
@@ -654,8 +654,6 @@ namespace dawn::native::metal {
         // Metal only allows format reinterpretation to happen on swizzle pattern or conversion
         // between linear space and sRGB. For example, creating bgra8Unorm texture view on
         // rgba8Unorm texture or creating rgba8Unorm_srgb texture view on rgab8Unorm texture.
-        // TODO: add MTLTextureUsagePixelFormatView when needed when we support other format
-        // reinterpretation.
         mtlDesc.usage = MetalTextureUsage(GetFormat(), GetInternalUsage(), GetSampleCount());
         mtlDesc.pixelFormat = MetalPixelFormat(GetFormat().format);
         mtlDesc.mipmapLevelCount = GetNumMipLevels();

src/dawn/native/vulkan/CommandBufferVk.cpp

@@ -668,7 +668,6 @@ namespace dawn::native::vulkan {
                                                   copy->copySize.depthOrArrayLayers));
                     }
 
-                    // TODO after Yunchao's CL
                     ToBackend(src.texture)
                         ->TransitionUsageNow(recordingContext, wgpu::TextureUsage::CopySrc,
                                              srcRange);

src/dawn/native/vulkan/SwapChainVk.cpp

@@ -309,7 +309,7 @@ namespace dawn::native::vulkan {
 
         DAWN_TRY_ASSIGN(mConfig, ChooseConfig(surfaceInfo));
 
-        // TODO Choose config instead of hardcoding
+        // TODO(dawn:269): Choose config instead of hardcoding
         VkSwapchainCreateInfoKHR createInfo;
         createInfo.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR;
         createInfo.pNext = nullptr;
@@ -514,7 +514,8 @@ namespace dawn::native::vulkan {
         CommandRecordingContext* recordingContext = device->GetPendingRecordingContext();
 
         if (mConfig.needsBlit) {
-            // TODO ditto same as present below: eagerly transition the blit texture to CopySrc.
+            // TODO(dawn:269): ditto same as present below: eagerly transition the blit texture to
+            // CopySrc.
             mBlitTexture->TransitionUsageNow(recordingContext, wgpu::TextureUsage::CopySrc,
                                              mBlitTexture->GetAllSubresources());
             mTexture->TransitionUsageNow(recordingContext, wgpu::TextureUsage::CopyDst,

src/dawn/node/binding/GPU.cpp

@@ -60,7 +60,7 @@ namespace wgpu::binding {
     // wgpu::bindings::GPU
     ////////////////////////////////////////////////////////////////////////////////
     GPU::GPU(Flags flags) : flags_(std::move(flags)) {
-        // TODO: Disable in 'release'
+        // TODO(dawn:1123): Disable in 'release'
         instance_.EnableBackendValidation(true);
         instance_.SetBackendValidationLevel(dawn::native::BackendValidationLevel::Full);
 

src/dawn/node/binding/GPUBuffer.cpp

@@ -97,8 +97,8 @@ namespace wgpu::binding {
                         break;
                     case WGPUBufferMapAsyncStatus_Unknown:
                     case WGPUBufferMapAsyncStatus_DeviceLost:
-                        // TODO: The spec is a bit vague around what the promise should do
-                        // here.
+                        // TODO(dawn:1123): The spec is a bit vague around what the promise should
+                        // do here.
                         c->promise.Reject(Errors::UnknownError(c->env));
                         break;
                 }

src/dawn/samples/ManualSwapChainTest.cpp

@@ -267,7 +267,7 @@ int main(int argc, const char* argv[]) {
     }
 
     // Choose an adapter we like.
-    // TODO: allow switching the window between devices.
+    // TODO(dawn:269): allow switching the window between devices.
     DawnProcTable procs = dawn::native::GetProcs();
     dawnProcSetProcs(&procs);
 

src/dawn/tests/end2end/DeviceLostTests.cpp

@@ -391,8 +391,8 @@ TEST_P(DeviceLostTest, GetMappedRange_MapAsyncWriting) {
     ASSERT_EQ(buffer.GetConstMappedRange(), rangeBeforeLoss);
 }
 
-// TODO mapasync read + resolve + loss getmappedrange != nullptr.
-// TODO mapasync write + resolve + loss getmappedrange != nullptr.
+// TODO(dawn:929): mapasync read + resolve + loss getmappedrange != nullptr.
+// TODO(dawn:929): mapasync write + resolve + loss getmappedrange != nullptr.
 
 // Test that Command Encoder Finish fails when device lost
 TEST_P(DeviceLostTest, CommandEncoderFinishFails) {

src/dawn/tests/end2end/SamplerTests.cpp

@@ -158,7 +158,6 @@ class SamplerTest : public DawnTest {
         EXPECT_PIXEL_RGBA8_EQ(expectedU3, mRenderPass.color, 3, 0);
         EXPECT_PIXEL_RGBA8_EQ(expectedV2, mRenderPass.color, 0, 2);
         EXPECT_PIXEL_RGBA8_EQ(expectedV3, mRenderPass.color, 0, 3);
-        // TODO: add tests for W address mode, once Dawn supports 3D textures
     }
 
     utils::BasicRenderPass mRenderPass;

src/dawn/tests/end2end/TextureSubresourceTests.cpp

@@ -188,14 +188,6 @@ TEST_P(TextureSubresourceTest, ArrayLayersTest) {
     EXPECT_TEXTURE_EQ(&bottomLeft, texture, {0, kSize - 1, 1}, {1, 1});
 }
 
-// TODO (yunchao.he@intel.com):
-// * add tests for storage texture and sampler across miplevel or
-// arraylayer dimensions in the same texture
-//
-// * add tests for copy operation upon texture subresource if needed
-//
-// * add tests for clear operation upon texture subresource if needed
-
 DAWN_INSTANTIATE_TEST(TextureSubresourceTest,
                       D3D12Backend(),
                       MetalBackend(),

src/dawn/tests/end2end/VertexStateTests.cpp

@@ -648,14 +648,6 @@ DAWN_INSTANTIATE_TEST(VertexStateTest,
                       OpenGLESBackend(),
                       VulkanBackend());
 
-// TODO for the input state:
-//  - Add more vertex formats
-//  - Add checks that the stride is enough to contain all attributes
-//  - Add checks stride less than some limit
-//  - Add checks for alignement of vertex buffers and attributes if needed
-//  - Check for attribute narrowing
-//  - Check that the input state and the pipeline vertex input types match
-
 class OptionalVertexStateTest : public DawnTest {};
 
 // Test that vertex input is not required in render pipeline descriptor.

src/dawn/tests/unittests/validation/ResourceUsageTrackingTests.cpp

@@ -1690,11 +1690,4 @@ namespace {
         }
     }
 
-    // TODO (yunchao.he@intel.com):
-    //
-    //  * Add tests for multiple encoders upon the same resource simultaneously. This situation fits
-    //    some cases like VR, multi-threading, etc.
-    //
-    //  * Add tests for bundle
-
 }  // anonymous namespace

src/dawn/tests/unittests/validation/TextureSubresourceTests.cpp

@@ -133,12 +133,4 @@ namespace {
         TestRenderPass(samplerView, renderView);
     }
 
-    // TODO (yunchao.he@intel.com):
-    //  * Add tests for compute, in which texture subresource is traced per dispatch.
-    //
-    //  * Add tests for multiple encoders upon the same resource simultaneously. This situation fits
-    //    some cases like VR, multi-threading, etc.
-    //
-    //  * Add tests for conflicts between usages in two render bundles used in the same pass.
-
 }  // anonymous namespace