- Reorder members for compactness.

- Avoid needing the definition of Error everywhere.
- use std::make_xxx and other c++11 constructs.

10 files changed, 42 insertions, 140 deletions

diff --git a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
index bc4fc80abad..fda3075fc28 100644
--- a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
+++ b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
@@ -1,6 +1,5 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <vespa/messagebus/testlib/receptor.h>
 #include <vespa/messagebus/testlib/simpleprotocol.h>
 #include <vespa/messagebus/testlib/simplemessage.h>
 #include <vespa/messagebus/testlib/simplereply.h>
@@ -24,6 +23,9 @@ Test::Main()
     SimpleProtocol protocol;
     EXPECT_TRUE(protocol.getName() == "Simple");
 
+    EXPECT_EQUAL(152u, sizeof(Result));
+    EXPECT_EQUAL(136u, sizeof(Error));
+    EXPECT_EQUAL(56u, sizeof(Routable));
     {
         // test protocol
         IRoutingPolicy::UP bogus = protocol.createPolicy("bogus", "");
@@ -32,7 +34,6 @@ Test::Main()
     TEST_FLUSH();
     {
         // test SimpleMessage
-        EXPECT_EQUAL(56u, sizeof(Routable));
         EXPECT_EQUAL(104u, sizeof(Message));
         EXPECT_EQUAL(184u, sizeof(SimpleMessage));
         auto msg = std::make_unique<SimpleMessage>("test");
@@ -52,7 +53,6 @@ Test::Main()
     TEST_FLUSH();
     {
         // test SimpleReply
-        EXPECT_EQUAL(56u, sizeof(Routable));
         EXPECT_EQUAL(96u, sizeof(Reply));
         EXPECT_EQUAL(160u, sizeof(SimpleReply));
         auto reply = std::make_unique<SimpleReply>("reply");
diff --git a/messagebus/src/vespa/messagebus/error.cpp b/messagebus/src/vespa/messagebus/error.cpp
index 85c9b92eb19..85eec140db5 100644
--- a/messagebus/src/vespa/messagebus/error.cpp
+++ b/messagebus/src/vespa/messagebus/error.cpp
@@ -3,7 +3,7 @@
 #include "errorcode.h"
 #include <vespa/vespalib/util/stringfmt.h>
 
-using vespalib::make_string;
+using vespalib::make_string_short::fmt;
 
 namespace mbus {
 
@@ -13,9 +13,9 @@ Error::Error()
       _service()
 { }
 
-Error::~Error() {}
+Error::~Error() = default;
 
-Error::Error(uint32_t c, const string &m, const string &s)
+Error::Error(uint32_t c, vespalib::stringref m, vespalib::stringref s)
     : _code(c),
       _msg(m),
       _service(s)
@@ -26,9 +26,9 @@ Error::toString() const
 {
     string name(ErrorCode::getName(_code));
     if (name.empty()) {
-        name = make_string("%u", _code);
+        name = fmt("%u", _code);
     }
-    return make_string("[%s @ %s]: %s", name.c_str(), _service.empty() ? "localhost" : _service.c_str(), _msg.c_str());
+    return fmt("[%s @ %s]: %s", name.c_str(), _service.empty() ? "localhost" : _service.c_str(), _msg.c_str());
 }
 
 } // namespace mbus
diff --git a/messagebus/src/vespa/messagebus/error.h b/messagebus/src/vespa/messagebus/error.h
index 638463e3665..74936962113 100644
--- a/messagebus/src/vespa/messagebus/error.h
+++ b/messagebus/src/vespa/messagebus/error.h
@@ -36,7 +36,7 @@ public:
      * @param m error message
      * @param s error service
      **/
-    Error(uint32_t c, const string &m, const string &s = "");
+    Error(uint32_t c, vespalib::stringref m, vespalib::stringref s = "");
 
     /**
      * Obtain the error code of this error.
diff --git a/messagebus/src/vespa/messagebus/message.cpp b/messagebus/src/vespa/messagebus/message.cpp
index 305ffb06aa0..8d17fd6b0ff 100644
--- a/messagebus/src/vespa/messagebus/message.cpp
+++ b/messagebus/src/vespa/messagebus/message.cpp
@@ -5,6 +5,7 @@
 #include "ireplyhandler.h"
 #include "emptyreply.h"
 #include "errorcode.h"
+#include "error.h"
 #include <vespa/vespalib/util/backtrace.h>
 
 #include <vespa/log/log.h>
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
index a8f7b47f3e3..775c90ea9ee 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
@@ -4,6 +4,7 @@
 #include "rpcnetwork.h"
 #include "rpcserviceaddress.h"
 #include <vespa/messagebus/emptyreply.h>
+#include <vespa/messagebus/error.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/fnet/frt/reflection.h>
 
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
index 16175fea1c2..a2e3310046d 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
@@ -4,6 +4,7 @@
 #include "rpcnetwork.h"
 #include "rpcserviceaddress.h"
 #include <vespa/messagebus/emptyreply.h>
+#include <vespa/messagebus/error.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/data/databuffer.h>
@@ -249,7 +250,7 @@ RPCSendV2::createResponse(FRT_Values & ret, const string & version, Reply & repl
             Cursor & error = array.addObject();
             error.setLong(CODE_F, reply.getError(i).getCode());
             error.setString(MSG_F, reply.getError(i).getMessage());
-            error.setString(SERVICE_F, reply.getError(i).getService().c_str());
+            error.setString(SERVICE_F, reply.getError(i).getService());
         }
     }
 
@@ -263,7 +264,6 @@ RPCSendV2::createResponse(FRT_Values & ret, const string & version, Reply & repl
     ret.AddInt32(toCompress.size());
     assert(buf.getDataLen() <= INT32_MAX);
     ret.AddData(std::move(buf));
-
 }
 
 } // namespace mbus
diff --git a/messagebus/src/vespa/messagebus/reply.cpp b/messagebus/src/vespa/messagebus/reply.cpp
index bf12698c86f..68bb7db1191 100644
--- a/messagebus/src/vespa/messagebus/reply.cpp
+++ b/messagebus/src/vespa/messagebus/reply.cpp
@@ -5,6 +5,7 @@
 #include "ireplyhandler.h"
 #include "message.h"
 #include "tracelevel.h"
+#include "error.h"
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vespalib/util/backtrace.h>
 
@@ -54,12 +55,6 @@ Reply::swapState(Routable &rhs)
     }
 }
 
-bool
-Reply::isReply() const
-{
-    return true;
-}
-
 void
 Reply::addError(const Error &e)
 {
@@ -72,16 +67,25 @@ Reply::addError(const Error &e)
 bool
 Reply::hasFatalErrors() const
 {
-    for (std::vector<Error>::const_iterator it = _errors.begin();
-         it != _errors.end(); ++it)
+    for (const Error & error : _errors)
     {
-        if (it->getCode() >= ErrorCode::FATAL_ERROR) {
+        if (error.getCode() >= ErrorCode::FATAL_ERROR) {
             return true;
         }
     }
     return false;
 }
 
+const Error &
+Reply::getError(uint32_t i) const {
+    return _errors[i];
+}
+
+uint32_t
+Reply::getNumErrors() const {
+    return _errors.size();
+}
+
 void
 Reply::setMessage(Message::UP msg) {
     _msg = std::move(msg);
diff --git a/messagebus/src/vespa/messagebus/reply.h b/messagebus/src/vespa/messagebus/reply.h
index 64b9f5c0b13..72493ec0ae6 100644
--- a/messagebus/src/vespa/messagebus/reply.h
+++ b/messagebus/src/vespa/messagebus/reply.h
@@ -1,13 +1,13 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include "error.h"
 #include "routable.h"
 #include <vector>
 
 namespace mbus {
 
 class Message;
+class Error;
 
 /**
  * A reply is a response to a message that has been sent throught the
@@ -43,7 +43,7 @@ public:
     ~Reply() override;
 
     void swapState(Routable &rhs) override;
-    bool isReply() const override;
+    bool isReply() const override { return true; }
 
     /**
      * Add an Error to this Reply
@@ -72,14 +72,14 @@ public:
      * @param i The index of the error to return.
      * @return The error at the given index.
      */
-    const Error &getError(uint32_t i) const { return _errors[i]; }
+    const Error &getError(uint32_t i) const;
 
     /**
      * Returns the number of errors that this reply contains.
      *
      * @return The number of replies.
      */
-    uint32_t getNumErrors() const { return _errors.size(); }
+    uint32_t getNumErrors() const;
 
     /**
      * Attach a Message to this Reply. If a Reply contains errors, messagebus
diff --git a/messagebus/src/vespa/messagebus/result.cpp b/messagebus/src/vespa/messagebus/result.cpp
index e99120d8c2d..970bb72e638 100644
--- a/messagebus/src/vespa/messagebus/result.cpp
+++ b/messagebus/src/vespa/messagebus/result.cpp
@@ -1,80 +1,22 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "result.h"
+#include "message.h"
 
 namespace mbus {
 
-Result::Handover::Handover(bool a, const Error &e, Message *m)
-    : _accepted(a),
-      _error(e),
-      _msg(m)
-{ }
-
 Result::Result()
     : _accepted(true),
       _error(),
       _msg()
 { }
 
-Result::Result(const Error &err, Message::UP msg)
+Result::Result(const Error &err, std::unique_ptr<Message> msg)
     : _accepted(false),
       _error(err),
       _msg(std::move(msg))
 { }
 
-Result::Result(Result &&rhs)
-    : _accepted(rhs._accepted),
-      _error(rhs._error),
-      _msg(std::move(rhs._msg))
-{ }
-
-Result::Result(const Handover &rhs)
-    : _accepted(rhs._accepted),
-      _error(rhs._error),
-      _msg(rhs._msg)
-{ }
-
-Result::~Result() {}
-
-bool
-Result::isAccepted() const
-{
-    return _accepted;
-}
-
-const Error &
-Result::getError() const
-{
-    return _error;
-}
-
-Message::UP
-Result::getMessage()
-{
-    return std::move(_msg);
-}
-
-Result::operator Handover()
-{
-    return Handover(_accepted, _error, _msg.release());
-}
-
-Result &
-Result::operator=(Result &&rhs)
-{
-    _accepted = rhs._accepted;
-    _error = rhs._error;
-    _msg = std::move(rhs._msg);
-    return *this;
-}
-
-Result &
-Result::operator=(const Handover &rhs)
-{
-    _accepted = rhs._accepted;
-    _error = rhs._error;
-    _msg.reset(rhs._msg);
-    return *this;
-}
+Result::~Result() = default;
 
 } // namespace mbus
diff --git a/messagebus/src/vespa/messagebus/result.h b/messagebus/src/vespa/messagebus/result.h
index adb9c1ea81a..dcf6e4964e5 100644
--- a/messagebus/src/vespa/messagebus/result.h
+++ b/messagebus/src/vespa/messagebus/result.h
@@ -3,10 +3,11 @@
 #pragma once
 
 #include "error.h"
-#include "message.h"
 
 namespace mbus {
 
+class Message;
+
 /**
  * A Result object is used as return value when trying to send a
  * Message on a SourceSession. It says whether messagebus has accepted
@@ -24,24 +25,10 @@ class Result
 private:
     bool        _accepted;
     Error       _error;
-    Message::UP _msg;
+    std::unique_ptr<Message> _msg;
 
 public:
     /**
-     * This inner class is used to implement destructive copy for
-     * return values.
-     **/
-    class Handover {
-        friend class Result;
-        bool      _accepted;
-        Error     _error;
-        Message  *_msg;
-        Handover(bool a, const Error &e, Message *m);
-        Handover(const Handover &); // not implemented
-        Handover &operator=(const Handover &); // not implemented
-    };
-
-    /**
      * Create a Result indicating that messagebus has accepted the
      * Message.
      **/
@@ -54,23 +41,11 @@ public:
      * @param err the reason for not accepting the Message
      * @param msg the message that did not get accepted
      **/
-    Result(const Error &err, Message::UP msg);
+    Result(const Error &err, std::unique_ptr<Message> msg);
 
-    /**
-     * Move constructor
-     *
-     * @param rhs the original object
-     **/
-    Result(Result &&rhs);
-
-    /**
-     * Construct a new Result from an internal Handover object that
-     * has destructed the original Result.
-     *
-     * @param rhs handover object
-     **/
-    Result(const Handover &rhs);
 
+    Result(Result &&rhs) = default;
+    Result &operator=(Result &&rhs) = default;
     ~Result();
 
     /**
@@ -78,14 +53,14 @@ public:
      *
      * @return true if the Message was accepted
      **/
-    bool isAccepted() const;
+    bool isAccepted() const { return _accepted; }
 
     /**
      * Obtain the error causing the message not to be accepted.
      *
      * @return error
      **/
-    const Error &getError() const;
+    const Error &getError() const { return _error; }
 
     /**
      * If the message was not accepted, this method may be used to get
@@ -95,28 +70,7 @@ public:
      *
      * @return the Message that was not accepted
      **/
-    Message::UP getMessage();
-
-    /**
-     * Perform an implicit typecast to support destructive copy of
-     * return values.
-     **/
-    operator Handover();
-
-    /**
-     * Moving assignment operator
-     *
-     * @param rhs the original object
-     **/
-    Result &operator=(Result &&rhs);
-
-    /**
-     * Assign a Result from an internal Handover object that has
-     * destructed the original Result.
-     *
-     * @param rhs handover object
-     **/
-    Result &operator=(const Handover &rhs);
+    std::unique_ptr<Message> getMessage() { return std::move(_msg); }
 };
 
 } // namespace mbus