Fix TextAdvance() problems involving whitespace around comment lines.

Fix the bug that TextAdvance() forgot to skip whitespace at the
beginning of the next line after a comment line.

Fix the bug that TextAdvanceLine() increase line number after going
over a character.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 710716b..d903f2d 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -157,9 +157,10 @@ if (TARGET gtest)
     ${CMAKE_CURRENT_SOURCE_DIR}/test/UnitSPIRV.h
 
     ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryEndianness.cpp
-    ${CMAKE_CURRENT_SOURCE_DIR}/test/FixWord.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryHeaderGet.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryToText.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/test/Comment.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/test/FixWord.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/LibspirvMacros.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/NamedId.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/OpcodeTableGet.cpp

diff --git a/source/text.cpp b/source/text.cpp
index f33d828..8fdcdcd 100644 (file)
--- a/source/text.cpp
+++ b/source/text.cpp
@@ -111,7 +111,7 @@ spv_result_t spvTextAdvanceLine(const spv_text text, spv_position position) {
         position->index++;
         return SPV_SUCCESS;
       default:
-        position->line++;
+        position->column++;
         position->index++;
         break;
     }
@@ -124,7 +124,8 @@ spv_result_t spvTextAdvance(const spv_text text, spv_position position) {
     case '\0':
       return SPV_END_OF_STREAM;
     case ';':
-      return spvTextAdvanceLine(text, position);
+      if (spv_result_t error = spvTextAdvanceLine(text, position)) return error;
+      return spvTextAdvance(text, position);
     case ' ':
     case '\t':
       position->column++;

diff --git a/test/Comment.cpp b/test/Comment.cpp
new file mode 100644 (file)
index 0000000..c323023
--- /dev/null
+++ b/test/Comment.cpp
@@ -0,0 +1,47 @@
+// Copyright (c) 2015 The Khronos Group Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and/or associated documentation files (the
+// "Materials"), to deal in the Materials without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Materials, and to
+// permit persons to whom the Materials are furnished to do so, subject to
+// the following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Materials.
+//
+// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS
+// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS
+// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT
+//    https://www.khronos.org/registry/
+//
+// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
+
+#include "TestFixture.h"
+#include "UnitSPIRV.h"
+
+TEST_F(TextToBinaryTest, Whitespace) {
+  SetText(R"(
+; I'm a proud comment at the begining of the file
+; I hide:   OpCapability Shader
+            OpMemoryModel Logical Simple ; comment after instruction
+;;;;;;;; many ;'s
+ %glsl450 = OpExtInstImport "GLSL.std.450"
+            ; comment indented
+)");
+  EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
+                                         extInstTable, &binary, &diagnostic));
+  if (binary) {
+    spvBinaryDestroy(binary);
+  }
+  if (diagnostic) {
+    spvDiagnosticPrint(diagnostic);
+  }
+}

diff --git a/test/TextAdvance.cpp b/test/TextAdvance.cpp
index 969a869..423ef9f 100644 (file)
--- a/test/TextAdvance.cpp
+++ b/test/TextAdvance.cpp
@@ -66,6 +66,23 @@ TEST(TextAdvance, LeadingNewLinesSpacesAndTabs) {
   ASSERT_EQ(5, position.index);
 }
 
+TEST(TextAdvance, LeadingWhitespaceAfterCommentLine) {
+  char textStr[] = "; comment\n \t \tWord";
+  spv_text_t text = {textStr, strlen(textStr)};
+  spv_position_t position = {};
+  ASSERT_EQ(SPV_SUCCESS, spvTextAdvance(&text, &position));
+  ASSERT_EQ(4, position.column);
+  ASSERT_EQ(1, position.line);
+  ASSERT_EQ(14, position.index);
+}
+
+TEST(TextAdvance, EOFAfterCommentLine) {
+  char textStr[] = "; comment";
+  spv_text_t text = {textStr, strlen(textStr)};
+  spv_position_t position = {};
+  ASSERT_EQ(SPV_END_OF_STREAM, spvTextAdvance(&text, &position));
+}
+
 TEST(TextAdvance, NullTerminator) {
   char textStr[] = "";
   spv_text_t text = {textStr, strlen(textStr)};

diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp
index f68ca66..8176415 100644 (file)
--- a/test/TextToBinary.cpp
+++ b/test/TextToBinary.cpp
@@ -47,15 +47,15 @@ TEST(TextToBinary, Default) {
       OpSourceExtension "PlaceholderExtensionName"
       OpEntryPoint Kernel $1
       OpExecutionMode $1 LocalSizeHint 1 1 1
-%2  = OpTypeVoid
-%3  = OpTypeBool
-; commment
-%4  = OpTypeInt 8 0 ; comment
-%5  = OpTypeInt 8 1
-%6  = OpTypeInt 16 0
-%7  = OpTypeInt 16 1
-%8  = OpTypeInt 32 0
-%9  = OpTypeInt 32 1
+ %2 = OpTypeVoid
+ %3 = OpTypeBool
+ ; commment
+ %4 = OpTypeInt 8 0 ; comment
+ %5 = OpTypeInt 8 1
+ %6 = OpTypeInt 16 0
+ %7 = OpTypeInt 16 1
+ %8 = OpTypeInt 32 0
+ %9 = OpTypeInt 32 1
 %10 = OpTypeInt 64 0
 %11 = OpTypeInt 64 1
 %12 = OpTypeFloat 16