From 4e2ed04d6e96db6b2b8e77c577c8439b23bc9a74 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Wed, 14 Sep 2016 12:48:38 -0400
Subject: [PATCH] Clarify: Ok to pass null diagnostic to spvBinaryParse

It's a valid use case: I might mostly trust my binaries and I don't
care to to collect detailed diagnostic feedback.
---
 include/spirv-tools/libspirv.h |  8 ++++----
 test/BinaryParse.cpp           | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h
index 878cc451..42ccb2a9 100644
--- a/include/spirv-tools/libspirv.h
+++ b/include/spirv-tools/libspirv.h
@@ -430,10 +430,10 @@ typedef spv_result_t (*spv_parsed_instruction_fn_t)(
 // callback once for each instruction in the stream.  The user_data parameter
 // is supplied as context to the callbacks.  Returns SPV_SUCCESS on successful
 // parse where the callbacks always return SPV_SUCCESS.  For an invalid parse,
-// returns a status code other than SPV_SUCCESS and emits a diagnostic.  If a
-// callback returns anything other than SPV_SUCCESS, then that status code
-// is returned, no further callbacks are issued, and no additional diagnostics
-// are emitted.
+// returns a status code other than SPV_SUCCESS, and if diagnostic is non-null
+// also emits a diagnostic.  If a callback returns anything other than
+// SPV_SUCCESS, then that status code is returned, no further callbacks are
+// issued, and no additional diagnostics are emitted.
 spv_result_t spvBinaryParse(const spv_const_context context, void* user_data,
                             const uint32_t* words, const size_t num_words,
                             spv_parsed_header_fn_t parse_header,
diff --git a/test/BinaryParse.cpp b/test/BinaryParse.cpp
index 0b6663cc..97c2104a 100644
--- a/test/BinaryParse.cpp
+++ b/test/BinaryParse.cpp
@@ -237,6 +237,27 @@ TEST_F(BinaryParseTest, EmptyModuleHasValidHeaderAndNoInstructionCallbacks) {
   }
 }
 
+TEST_F(BinaryParseTest, NullDiagnosticsIsOkForGoodParse) {
+  const auto words = CompileSuccessfully("");
+  EXPECT_HEADER(1).WillOnce(Return(SPV_SUCCESS));
+  EXPECT_CALL(client_, Instruction(_)).Times(0);  // No instruction callback.
+  EXPECT_EQ(
+      SPV_SUCCESS,
+      spvBinaryParse(ScopedContext().context, &client_, words.data(),
+                     words.size(), invoke_header, invoke_instruction, nullptr));
+}
+
+TEST_F(BinaryParseTest, NullDiagnosticsIsOkForBadParse) {
+  auto words = CompileSuccessfully("");
+  words.push_back(0xffffffff);  // Certainly invalid instruction header.
+  EXPECT_HEADER(1).WillOnce(Return(SPV_SUCCESS));
+  EXPECT_CALL(client_, Instruction(_)).Times(0);  // No instruction callback.
+  EXPECT_EQ(
+      SPV_ERROR_INVALID_BINARY,
+      spvBinaryParse(ScopedContext().context, &client_, words.data(),
+                     words.size(), invoke_header, invoke_instruction, nullptr));
+}
+
 TEST_F(BinaryParseTest,
        ModuleWithSingleInstructionHasValidHeaderAndInstructionCallback) {
   for (bool endian_swap : kSwapEndians) {