From 7a9fe30b11f454f669d7bfbb1ba15a45f0678efa Mon Sep 17 00:00:00 2001
From: Corentin Wallez <cwallez@chromium.org>
Date: Wed, 23 Nov 2022 17:20:44 +0000
Subject: [PATCH] CompilationMessages: compute offsets directly to take \r\n
 into account.

Bug: dawn:1357
Fixed: tint:1684
Change-Id: Ia1e9af3ff3ccb0fe674a838b966fa8a49ff8fb4f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111060
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
---
 src/dawn/native/CompilationMessages.cpp | 53 ++++++++++---------------
 webgpu-cts/expectations.txt             |  1 -
 2 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/src/dawn/native/CompilationMessages.cpp b/src/dawn/native/CompilationMessages.cpp
index a605b299f8..dd4fd41f35 100644
--- a/src/dawn/native/CompilationMessages.cpp
+++ b/src/dawn/native/CompilationMessages.cpp
@@ -64,49 +64,36 @@ void OwnedCompilationMessages::AddMessage(const tint::diag::Diagnostic& diagnost
 
     // Tint line and column values are 1-based.
     uint64_t lineNum = diagnostic.source.range.begin.line;
-    uint64_t linePos = diagnostic.source.range.begin.column;
+    uint64_t lineCol = diagnostic.source.range.begin.column;
     // The offset is 0-based.
     uint64_t offset = 0;
     uint64_t length = 0;
 
-    if (lineNum && linePos && diagnostic.source.file) {
-        const auto& lines = diagnostic.source.file->content.lines;
-        size_t i = 0;
-        // To find the offset of the message position, loop through each of the first lineNum-1
-        // lines and add it's length (+1 to account for the line break) to the offset.
-        for (; i < lineNum - 1; ++i) {
-            offset += lines[i].length() + 1;
-        }
+    if (lineNum && lineCol && diagnostic.source.file) {
+        const tint::Source::FileContent& content = diagnostic.source.file->content;
 
-        // If the end line is on a different line from the beginning line, add the length of the
-        // lines in between to the ending offset.
+        // Tint stores line as std::string_view in a complete source std::string that's in the
+        // source file. So to get the offset in bytes of a line we just need to substract its start
+        // pointer with the start of the file's content. Note that line numbering in Tint source
+        // range starts at 1 while the array of lines start at 0 (hence the -1).
+        const char* fileStart = content.data.data();
+        const char* lineStart = content.lines[lineNum - 1].data();
+        offset = static_cast<uint64_t>(lineStart - fileStart) + lineCol - 1;
+
+        // If the range has a valid start but the end is not specified, clamp it to the start.
         uint64_t endLineNum = diagnostic.source.range.end.line;
-        uint64_t endLinePos = diagnostic.source.range.end.column;
-
-        // If the range has a valid start but the end it not specified, clamp it to the start.
-        if (endLineNum == 0 || endLinePos == 0) {
+        uint64_t endLineCol = diagnostic.source.range.end.column;
+        if (endLineNum == 0 || endLineCol == 0) {
             endLineNum = lineNum;
-            endLinePos = linePos;
+            endLineCol = lineCol;
         }
 
-        // Negative ranges aren't allowed
-        ASSERT(endLineNum >= lineNum);
-
-        uint64_t endOffset = offset;
-        for (; i < endLineNum - 1; ++i) {
-            endOffset += lines[i].length() + 1;
-        }
-
-        // Add the line positions to the offset and endOffset to get their final positions
-        // within the code string.
-        offset += linePos - 1;
-        endOffset += endLinePos - 1;
-
-        // Negative ranges aren't allowed
-        ASSERT(endOffset >= offset);
+        const char* endLineStart = content.lines[endLineNum - 1].data();
+        uint64_t endOffset = static_cast<uint64_t>(endLineStart - fileStart) + endLineCol - 1;
 
         // The length of the message is the difference between the starting offset and the
-        // ending offset.
+        // ending offset. Negative ranges aren't allowed
+        ASSERT(endOffset >= offset);
         length = endOffset - offset;
     }
 
@@ -117,7 +104,7 @@ void OwnedCompilationMessages::AddMessage(const tint::diag::Diagnostic& diagnost
     }
 
     mMessages.push_back({nullptr, nullptr, tintSeverityToMessageType(diagnostic.severity), lineNum,
-                         linePos, offset, length});
+                         lineCol, offset, length});
 }
 
 void OwnedCompilationMessages::AddMessages(const tint::diag::List& diagnostics) {
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index 29ebf86cb0..b0921d3838 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -202,7 +202,6 @@ crbug.com/dawn/1125 [ ubuntu ] webgpu:api,operation,rendering,depth_clip_clamp:d
 ################################################################################
 # compilation_info failures
 ################################################################################
-crbug.com/dawn/1357 webgpu:api,operation,shader_module,compilation_info:offset_and_length:valid=false;name="carriage-return" [ Failure ]
 crbug.com/dawn/1357 webgpu:api,operation,shader_module,compilation_info:offset_and_length:valid=false;name="unicode" [ Failure ]
 
 ################################################################################