Skip to content

Commit

Permalink
DiagnosticStream move ctor moves output duties to new object
Browse files Browse the repository at this point in the history
- Take over contents of the expiring message stream
- Prevent the expiring object from emitting anything during destruction
  • Loading branch information
dneto0 authored and antiagainst committed Oct 3, 2017
1 parent 17a843c commit 169266e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 7 deletions.
14 changes: 14 additions & 0 deletions source/diagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <cassert>
#include <cstring>
#include <iostream>
#include <sstream>

#include "table.h"

Expand Down Expand Up @@ -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;
Expand Down
16 changes: 9 additions & 7 deletions source/diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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_;
};

Expand Down
26 changes: 26 additions & 0 deletions test/diagnostic_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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

0 comments on commit 169266e

Please sign in to comment.