From ea633a6427e05f2f5c266b19a239b3306887b78d Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Mon, 2 Nov 2015 11:40:59 -0500
Subject: [PATCH] Empty assembly text compiles to no instructions.

But it's still a valid module.
---
 source/text.cpp       |  7 ++++---
 test/Comment.cpp      | 23 +++++++++++++++--------
 test/TestFixture.h    |  2 +-
 test/TextToBinary.cpp | 10 ++++++++++
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/source/text.cpp b/source/text.cpp
index ef90cb1..a79699d 100644
--- a/source/text.cpp
+++ b/source/text.cpp
@@ -642,8 +642,8 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
                                      spv_diagnostic* pDiagnostic) {
   if (!pDiagnostic) return SPV_ERROR_INVALID_DIAGNOSTIC;
   libspirv::AssemblyContext context(text, pDiagnostic);
-  if (!text->str || !text->length)
-    return context.diagnostic() << "Text stream is empty.";
+  if (!text->str)
+    return context.diagnostic() << "Missing assembly text.";
 
   if (!grammar.isValid()) {
     return SPV_ERROR_INVALID_TABLE;
@@ -655,7 +655,8 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
 
   std::vector<spv_instruction_t> instructions;
 
-  if (context.advance()) return context.diagnostic() << "Text stream is empty.";
+  // Skip past whitespace and comments.
+  context.advance();
 
   spv_ext_inst_type_t extInstType = SPV_EXT_INST_TYPE_NONE;
   while (context.hasText()) {
diff --git a/test/Comment.cpp b/test/Comment.cpp
index f3a29b6..de023c7 100644
--- a/test/Comment.cpp
+++ b/test/Comment.cpp
@@ -24,28 +24,35 @@
 // TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
 // MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
 
+#include "gmock/gmock.h"
 #include "TestFixture.h"
 #include "UnitSPIRV.h"
 
 namespace {
 
+using spvtest::Concatenate;
+using spvtest::MakeInstruction;
+using spvtest::MakeVector;
 using spvtest::TextToBinaryTest;
+using testing::Eq;
 
 TEST_F(TextToBinaryTest, Whitespace) {
-  SetText(R"(
+  std::string input = 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.str, text.length, opcodeTable, operandTable,
-                            extInstTable, &binary, &diagnostic));
-  if (diagnostic) {
-    spvDiagnosticPrint(diagnostic);
-  }
+)";
+
+  EXPECT_THAT(
+      CompiledInstructions(input),
+      Eq(Concatenate({MakeInstruction(SpvOpMemoryModel,
+                                      {uint32_t(SpvAddressingModelLogical),
+                                       uint32_t(SpvMemoryModelSimple)}),
+                      MakeInstruction(SpvOpExtInstImport, {1},
+                                      MakeVector("GLSL.std.450"))})));
 }
 
 }  // anonymous namespace
diff --git a/test/TestFixture.h b/test/TestFixture.h
index 7ebca8e..02eb6cd 100644
--- a/test/TestFixture.h
+++ b/test/TestFixture.h
@@ -62,7 +62,7 @@ class TextToBinaryTestBase : public T {
 
   // Returns subvector v[from:end).
   SpirvVector Subvector(const SpirvVector& v, SpirvVector::size_type from) {
-    assert(from < v.size());
+    assert(from <= v.size());
     return SpirvVector(v.begin() + from, v.end());
   }
 
diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp
index cf8573b..61020d2 100644
--- a/test/TextToBinary.cpp
+++ b/test/TextToBinary.cpp
@@ -25,6 +25,7 @@
 // MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
 
 #include "TestFixture.h"
+#include "gmock/gmock.h"
 #include "UnitSPIRV.h"
 #include "util/bitutils.h"
 
@@ -38,6 +39,7 @@ using libspirv::AssemblyContext;
 using libspirv::AssemblyGrammar;
 using spvtest::TextToBinaryTest;
 using spvtest::AutoText;
+using testing::Eq;
 
 TEST(GetWord, Simple) {
   EXPECT_EQ("", AssemblyContext(AutoText(""), nullptr).getWord());
@@ -302,6 +304,8 @@ TEST_F(TextToBinaryTest, InvalidText) {
   ASSERT_EQ(SPV_ERROR_INVALID_TEXT,
             spvTextToBinary(nullptr, 0, opcodeTable, operandTable, extInstTable,
                             &binary, &diagnostic));
+  EXPECT_NE(nullptr, diagnostic);
+  EXPECT_THAT(diagnostic->error, Eq(std::string("Missing assembly text.")));
 }
 
 TEST_F(TextToBinaryTest, InvalidTable) {
@@ -345,6 +349,12 @@ TEST_F(TextToBinaryTest, InvalidPrefix) {
   }
 }
 
+TEST_F(TextToBinaryTest, EmptyAssemblyString) {
+  // An empty assembly module is valid!
+  // It should produce a valid module with zero instructions.
+  EXPECT_THAT(CompiledInstructions(""), Eq(std::vector<uint32_t>{}));
+}
+
 TEST_F(TextToBinaryTest, StringSpace) {
   SetText("OpSourceExtension \"string with spaces\"");
   EXPECT_EQ(SPV_SUCCESS,
-- 
2.7.4