DiagnosticStream move ctor moves output duties to new object

- Take over contents of the expiring message stream
- Prevent the expiring object from emitting anything during destruction

diff --git a/source/diagnostic.cpp b/source/diagnostic.cpp
index b887c7adaf4ddc32ff7711bc6076586cce74fda1..44339f69e5aa8080765cea5434373ebc90b9d9f6 100644 (file)
--- a/source/diagnostic.cpp
+++ b/source/diagnostic.cpp
@@ -17,6 +17,7 @@
 #include <cassert>
 #include <cstring>
 #include <iostream>
+#include <sstream>
 
 #include "table.h"
 
@@ -66,6 +67,19 @@ spv_result_t spvDiagnosticPrint(const spv_diagnostic diagnostic) {
 
 namespace libspirv {
 
+DiagnosticStream::DiagnosticStream(DiagnosticStream&& other)
+    : stream_(),
+      position_(other.position_),
+      consumer_(other.consumer_),
+      error_(other.error_) {
+  // Prevent the other object from emitting output during destruction.
+  other.error_ = SPV_FAILED_MATCH;
+  // Some platforms are missing support for std::ostringstream functionality,
+  // including:  move constructor, swap method.  Either would have been a
+  // better choice than copying the string.
+  stream_ << other.stream_.str();
+}
+
 DiagnosticStream::~DiagnosticStream() {
   if (error_ != SPV_FAILED_MATCH && consumer_ != nullptr) {
     auto level = SPV_MSG_ERROR;

diff --git a/source/diagnostic.h b/source/diagnostic.h
index 78f9a5b65a0b08f31261feb0d418af0a012b40ae..3d06f8df002b2d55ebb9da7a7afc7ddb64e5e470 100644 (file)
--- a/source/diagnostic.h
+++ b/source/diagnostic.h
@@ -33,12 +33,14 @@ class DiagnosticStream {
                    spv_result_t error)
       : position_(position), consumer_(consumer), error_(error) {}
 
-  DiagnosticStream(DiagnosticStream&& other)
-      : stream_(other.stream_.str()),
-        position_(other.position_),
-        consumer_(other.consumer_),
-        error_(other.error_) {}
+  // Creates a DiagnosticStream from an expiring DiagnosticStream.
+  // The new object takes the contents of the other, and prevents the
+  // other from emitting anything during destruction.
+  DiagnosticStream(DiagnosticStream&& other);
 
+  // Destroys a DiagnosticStream.
+  // If its status code is something other than SPV_FAILED_MATCH
+  // then emit the accumulated message to the consumer.
   ~DiagnosticStream();
 
   // Adds the given value to the diagnostic message to be written.
@@ -52,9 +54,9 @@ class DiagnosticStream {
   operator spv_result_t() { return error_; }
 
  private:
-  std::stringstream stream_;
+  std::ostringstream stream_;
   spv_position_t position_;
-  const spvtools::MessageConsumer& consumer_;  // Message consumer callback.
+  spvtools::MessageConsumer consumer_;  // Message consumer callback.
   spv_result_t error_;
 };
 

diff --git a/test/diagnostic_test.cpp b/test/diagnostic_test.cpp
index 65af274dd86142b3117b7051f7c41a20bc5fb04f..f023730477b106f8d794bbb819d6b13b51aff5a6 100644 (file)
--- a/test/diagnostic_test.cpp
+++ b/test/diagnostic_test.cpp
@@ -12,11 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <algorithm>
+#include <sstream>
+
+#include "gmock/gmock.h"
 #include "unit_spirv.h"
 
 namespace {
 
 using libspirv::DiagnosticStream;
+using ::testing::Eq;
 
 // Returns a newly created diagnostic value.
 spv_diagnostic MakeValidDiagnostic() {
@@ -75,4 +80,25 @@ TEST(DiagnosticStream, ConversionToResultType) {
             spv_result_t(DiagnosticStream({}, nullptr, SPV_FAILED_MATCH)));
 }
 
+TEST(DiagnosticStream, MoveConstructorPreservesPreviousMessagesAndPreventsOutputFromExpiringValue) {
+  std::ostringstream messages;
+  int message_count = 0;
+  auto consumer = [&messages, &message_count](spv_message_level_t, const char*,
+                                              const spv_position_t&,
+                                              const char* msg) {
+    message_count++;
+    messages << msg;
+  };
+
+  // Enclose the DiagnosticStream variables in a scope to force destruction.
+  {
+    DiagnosticStream ds0({}, consumer, SPV_ERROR_INVALID_BINARY);
+    ds0 << "First";
+    DiagnosticStream ds1(std::move(ds0));
+    ds1 << "Second";
+  }
+  EXPECT_THAT(message_count, Eq(1));
+  EXPECT_THAT(messages.str(), Eq("FirstSecond"));
+}
+
 }  // anonymous namespace