Bugfix: report the correct location for wrong opcode.

Also add more tests for the "<result-id> = <opcode> <operand>.."
format.

diff --git a/source/text.cpp b/source/text.cpp
index 8fdcdcd..1c91276 100644 (file)
--- a/source/text.cpp
+++ b/source/text.cpp
@@ -540,10 +540,11 @@ spv_result_t spvTextEncodeOpcode(
     spvCheck(spvTextAdvance(text, position),
              DIAGNOSTIC << "Expected opcode, found end of stream.";
              return SPV_ERROR_INVALID_TEXT);
+    error = spvTextWordGet(text, position, opcodeName, &nextPosition);
+    spvCheck(error, return error);
     spvCheck(!spvStartsWithOp(text, position),
              DIAGNOSTIC << "Invalid Opcode prefix '" << opcodeName << "'.";
              return SPV_ERROR_INVALID_TEXT);
-    error = spvTextWordGet(text, position, opcodeName, &nextPosition);
   }
 
   // NOTE: The table contains Opcode names without the "Op" prefix.

diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp
index 8176415..7af8cb0 100644 (file)
--- a/test/TextToBinary.cpp
+++ b/test/TextToBinary.cpp
@@ -343,6 +343,39 @@ TEST_F(TextToBinaryTest, StringSpace) {
   }
 }
 
+// TODO(antiagainst): we might not want to support both instruction formats in
+// the future. Only the "<result-id> = <opcode> <operand>.." one may survive.
+TEST_F(TextToBinaryTest, InstructionTwoFormats) {
+  SetText(R"(
+            OpCapability Shader
+ %glsl450 = OpExtInstImport "GLSL.std.450"
+            OpMemoryModel Logical Simple
+            OpTypeBool %3
+       %4 = OpTypeInt 8 0
+            OpTypeInt %5 8 1
+       %6 = OpTypeInt 16 0
+            OpTypeInt %7 16 1
+    %void = OpTypeVoid
+            OpTypeFloat %float 32
+%const1.5 = OpConstant $float 1.5
+            OpTypeFunction %fnMain $void
+    %main = OpFunction $void None $fnMain
+            OpLabel %lbMain
+  %result = OpExtInst $float $glsl450 Round $const1.5
+            OpReturn
+            OpFunctionEnd
+)");
+
+  EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
+                                         extInstTable, &binary, &diagnostic));
+  if (binary) {
+    spvBinaryDestroy(binary);
+  }
+  if (diagnostic) {
+    spvDiagnosticPrint(diagnostic);
+  }
+}
+
 TEST_F(TextToBinaryTest, UnknownBeginningOfInsruction) {
   SetText(R"(
      OpSource OpenCL 12
@@ -361,3 +394,51 @@ Google
       diagnostic->error);
   if (binary) spvBinaryDestroy(binary);
 }
+
+TEST_F(TextToBinaryTest, NoEqualSign) {
+  SetText(R"(
+     OpSource OpenCL 12
+     OpMemoryModel Physical64 OpenCL
+%2
+)");
+
+  EXPECT_EQ(SPV_ERROR_INVALID_TEXT,
+            spvTextToBinary(&text, opcodeTable, operandTable, extInstTable,
+                            &binary, &diagnostic));
+  EXPECT_EQ(5, diagnostic->position.line + 1);
+  EXPECT_EQ(1, diagnostic->position.column + 1);
+  EXPECT_STREQ("Expected '=', found end of stream.", diagnostic->error);
+  if (binary) spvBinaryDestroy(binary);
+}
+
+TEST_F(TextToBinaryTest, NoOpCode) {
+  SetText(R"(
+     OpSource OpenCL 12
+     OpMemoryModel Physical64 OpenCL
+%2 =
+)");
+
+  EXPECT_EQ(SPV_ERROR_INVALID_TEXT,
+            spvTextToBinary(&text, opcodeTable, operandTable, extInstTable,
+                            &binary, &diagnostic));
+  EXPECT_EQ(5, diagnostic->position.line + 1);
+  EXPECT_EQ(1, diagnostic->position.column + 1);
+  EXPECT_STREQ("Expected opcode, found end of stream.", diagnostic->error);
+  if (binary) spvBinaryDestroy(binary);
+}
+
+TEST_F(TextToBinaryTest, WrongOpCode) {
+  SetText(R"(
+     OpSource OpenCL 12
+     OpMemoryModel Physical64 OpenCL
+%2 = Wahahaha
+)");
+
+  EXPECT_EQ(SPV_ERROR_INVALID_TEXT,
+            spvTextToBinary(&text, opcodeTable, operandTable, extInstTable,
+                            &binary, &diagnostic));
+  EXPECT_EQ(4, diagnostic->position.line + 1);
+  EXPECT_EQ(6, diagnostic->position.column + 1);
+  EXPECT_STREQ("Invalid Opcode prefix 'Wahahaha'.", diagnostic->error);
+  if (binary) spvBinaryDestroy(binary);
+}