Make version class data members public

3 files changed, 109 insertions, 76 deletions

diff --git a/bpkg/manifest b/bpkg/manifest
index aab1c91..8f5d975 100644
--- a/bpkg/manifest
+++ b/bpkg/manifest
@@ -26,46 +26,41 @@ namespace bpkg
   class version
   {
   public:
+    // Let's keep the members in the order they appear in the string
+    // representation.
+    //
+    const std::uint16_t epoch;
+    const std::string upstream;
+    const std::uint16_t revision;
+
+    // Upstream part canonical representation.
+    //
+    const std::string canonical_upstream;
+
     // Create a special empty version.
     //
-    version (): epoch_ (0), revision_ (0) {}
+    version (): epoch (0), revision (0) {}
 
     // Throw std::invalid_argument if the passed string is not a valid
     // version representation.
     //
     explicit
-    version (const std::string& v): version (v.c_str ()) /* Delegate */ {}
+    version (const std::string& v): version (v.c_str ()) {}
 
     explicit
-    version (const char* v): version (v, false) /* Delegate */ {}
+    version (const char* v): version (data_type (v, false)) {}
 
     // Create the version object from separate epoch, upstream, and
     // revision parts.
     //
     version (std::uint16_t epoch,
              std::string upstream,
-             std::uint16_t revision)
-        : version (upstream.c_str (), true)  // Delegate
-
-    {
-      // Can't initialize in member initializer list due to construction
-      // delegation.
-      //
-      epoch_ = epoch;
-      revision_ = revision;
-    }
-
-    std::uint16_t
-    epoch () const noexcept {return epoch_;}
+             std::uint16_t revision);
 
-    std::uint16_t
-    revision () const noexcept {return revision_;}
-
-    const std::string&
-    upstream () const noexcept {return upstream_;}
-
-    const std::string&
-    canonical_upstream () const noexcept {return canonical_upstream_;}
+    version (version&&) = default;
+    version (const version&) = default;
+    version& operator= (version&&);
+    version& operator= (const version&);
 
     std::string
     string (bool ignore_revision = false) const;
@@ -91,14 +86,14 @@ namespace bpkg
     int
     compare (const version& v, bool ignore_revision = false) const noexcept
     {
-      if (epoch_ != v.epoch_)
-        return epoch_ < v.epoch_ ? -1 : 1;
+      if (epoch != v.epoch)
+        return epoch < v.epoch ? -1 : 1;
 
-      if (int c = canonical_upstream_.compare (v.canonical_upstream_))
+      if (int c = canonical_upstream.compare (v.canonical_upstream))
         return c;
 
-      if (!ignore_revision && revision_ != v.revision_)
-        return revision_ < v.revision_ ? -1 : 1;
+      if (!ignore_revision && revision != v.revision)
+        return revision < v.revision ? -1 : 1;
 
       return 0;
     }
@@ -110,20 +105,26 @@ namespace bpkg
       // version object can not have them different from 0 if upstream is
       // empty.
       //
-      return upstream_.empty ();
+      return upstream.empty ();
     }
 
   private:
-    version (const char*, bool upstream_only);
+    struct data_type
+    {
+      data_type (const char*, bool upstream_only);
 
-  private:
-    // Let's keep the members in the order they appear in the string
-    // representation.
-    //
-    std::uint16_t epoch_;
-    std::string upstream_;
-    std::uint16_t revision_;
-    std::string canonical_upstream_; // Upstream part canonical representation.
+      std::uint16_t epoch;
+      std::string upstream;
+      std::uint16_t revision;
+      std::string canonical_upstream;
+    };
+
+    explicit
+    version (data_type&& d)
+        : epoch (d.epoch),
+          upstream (std::move (d.upstream)),
+          revision (d.revision),
+          canonical_upstream (std::move (d.canonical_upstream)) {}
   };
 
   inline std::ostream&
diff --git a/bpkg/manifest.cxx b/bpkg/manifest.cxx
index 122f781..cc0065b 100644
--- a/bpkg/manifest.cxx
+++ b/bpkg/manifest.cxx
@@ -178,7 +178,17 @@ namespace bpkg
   // version
   //
   version::
-  version (const char* v, bool upstream_only): version () // Delegate
+  version (uint16_t e, std::string u, uint16_t r)
+      : epoch (e),
+        upstream (move (u)),
+        revision (r),
+        canonical_upstream (
+          data_type (upstream.c_str (), true).canonical_upstream)
+  {
+  }
+
+  version::data_type::
+  data_type (const char* v, bool upstream_only): epoch (0), revision (0)
   {
     // Otherwise compiler gets confused with string() member.
     //
@@ -202,30 +212,32 @@ namespace bpkg
     auto add_canonical_component (
       [this, &bad_arg](const char* b, const char* e, bool numeric) -> bool
       {
-        if (!canonical_upstream_.empty ())
-          canonical_upstream_.append (1, '.');
+        auto& cu (canonical_upstream);
+
+        if (!cu.empty ())
+          cu.append (1, '.');
 
         if (numeric)
         {
           if (e - b > 8)
             bad_arg ("8 digits maximum allowed in a component");
 
-          canonical_upstream_.append (8 - (e - b), '0'); // Add padding spaces.
+          cu.append (8 - (e - b), '0'); // Add padding spaces.
 
           string c (b, e);
-          canonical_upstream_.append (c);
+          cu.append (c);
           return stoul (c) != 0;
         }
         else
         {
           for (const char* i (b); i != e; ++i)
-            canonical_upstream_.append (1, lowercase (*i));
+            cu.append (1, lowercase (*i));
 
           return true;
         }
       });
 
-    enum {epoch, upstream, revision} mode (epoch);
+    enum class mode {epoch, upstream, revision} m (mode::epoch);
 
     const char* cb (v); // Begin of a component.
     const char* ub (v); // Begin of upstream component.
@@ -247,14 +259,14 @@ namespace bpkg
           if (upstream_only)
             bad_arg ("unexpected '+' character");
 
-          if (mode != epoch || p == v)
+          if (m != mode::epoch || p == v)
             bad_arg ("unexpected '+' character position");
 
           if (lnn >= cb) // Contains non-digits.
             bad_arg ("epoch should be 2-byte unsigned integer");
 
-          epoch_ = uint16 (string (cb, p), "epoch");
-          mode = upstream;
+          epoch = uint16 (string (cb, p), "epoch");
+          m = mode::upstream;
           cb = p + 1;
           ub = cb;
           break;
@@ -270,14 +282,14 @@ namespace bpkg
 
       case '.':
         {
-          if ((mode != epoch && mode != upstream) || p == cb)
+          if ((m != mode::epoch && m != mode::upstream) || p == cb)
             bad_arg (string ("unexpected '") + c + "' character position");
 
           if (add_canonical_component (cb, p, lnn < cb))
-            cl = canonical_upstream_.size ();
+            cl = canonical_upstream.size ();
 
           ue = p;
-          mode = c == '-' ? revision : upstream;
+          m = c == '-' ? mode::revision : mode::upstream;
           cb = p + 1;
           break;
         }
@@ -295,37 +307,57 @@ namespace bpkg
     if (p == cb)
       bad_arg ("unexpected end");
 
-    if (mode == revision)
+    if (m == mode::revision)
     {
       if (lnn >= cb) // Contains non-digits.
         bad_arg ("revision should be 2-byte unsigned integer");
 
-      revision_ = uint16 (cb, "revision");
+      revision = uint16 (cb, "revision");
     }
     else
     {
       if (add_canonical_component (cb, p, lnn < cb))
-        cl = canonical_upstream_.size ();
+        cl = canonical_upstream.size ();
 
       ue = p;
     }
 
     assert (ub != ue); // Can't happen if through all previous checks.
-    upstream_.assign (ub, ue);
-    canonical_upstream_.resize (cl);
+
+    if (!upstream_only)
+      upstream.assign (ub, ue);
+
+    canonical_upstream.resize (cl);
+  }
+
+  version& version::
+  operator= (const version& v)
+  {
+    if (this != &v)
+      *this = version (v); // Reduce to move-assignment.
+    return *this;
+  }
+
+  version& version::
+  operator= (version&& v)
+  {
+    if (this != &v)
+    {
+      this->~version ();
+      new (this) version (move (v)); // Assume noexcept move-construction.
+    }
+    return *this;
   }
 
   string version::
   string (bool ignore_revision) const
   {
-    std::string v (epoch_ != 0
-                   ? to_string (epoch_) + "+" + upstream_
-                   : upstream_);
+    std::string v (epoch != 0 ? to_string (epoch) + "+" + upstream : upstream);
 
-    if (!ignore_revision && revision_ != 0)
+    if (!ignore_revision && revision != 0)
     {
       v += '-';
-      v += to_string (revision_);
+      v += to_string (revision);
     }
 
     return v;
diff --git a/tests/package-version/driver.cxx b/tests/package-version/driver.cxx
index d9a54d0..df4d73b 100644
--- a/tests/package-version/driver.cxx
+++ b/tests/package-version/driver.cxx
@@ -45,7 +45,7 @@ bad_version (uint16_t e, const string& u, uint16_t r)
 static bool
 test_constructor (const version& v)
 {
-  return v == version (v.epoch (), v.upstream (), v.revision ());
+  return v == version (v.epoch, v.upstream, v.revision);
 }
 
 int
@@ -89,91 +89,91 @@ main (int argc, char* argv[])
     {
       version v ("a");
       assert (v.string () == "a");
-      assert (v.canonical_upstream () == "a");
+      assert (v.canonical_upstream == "a");
       assert (test_constructor (v));
     }
 
     {
       version v ("65535+ab-65535");
       assert (v.string () == "65535+ab-65535");
-      assert (v.canonical_upstream () == "ab");
+      assert (v.canonical_upstream == "ab");
       assert (test_constructor (v));
     }
 
     {
       version v ("1");
       assert (v.string () == "1");
-      assert (v.canonical_upstream () == "00000001");
+      assert (v.canonical_upstream == "00000001");
       assert (test_constructor (v));
     }
 
     {
       version v ("0");
       assert (v.string () == "0");
-      assert (v.canonical_upstream ().empty ());
+      assert (v.canonical_upstream.empty ());
       assert (test_constructor (v));
     }
 
     {
       version v ("0.0.0");
       assert (v.string () == "0.0.0");
-      assert (v.canonical_upstream ().empty ());
+      assert (v.canonical_upstream.empty ());
       assert (test_constructor (v));
     }
 
     {
       version v ("1.0.0");
       assert (v.string () == "1.0.0");
-      assert (v.canonical_upstream () == "00000001");
+      assert (v.canonical_upstream == "00000001");
       assert (test_constructor (v));
     }
 
     {
       version v ("0.1.00");
       assert (v.string () == "0.1.00");
-      assert (v.canonical_upstream () == "00000000.00000001");
+      assert (v.canonical_upstream == "00000000.00000001");
       assert (test_constructor (v));
     }
 
     {
       version v ("0.0a.00");
       assert (v.string () == "0.0a.00");
-      assert (v.canonical_upstream () == "00000000.0a");
+      assert (v.canonical_upstream == "00000000.0a");
       assert (test_constructor (v));
     }
 
     {
       version v ("0.a00.00");
       assert (v.string () == "0.a00.00");
-      assert (v.canonical_upstream () == "00000000.a00");
+      assert (v.canonical_upstream == "00000000.a00");
       assert (test_constructor (v));
     }
 
     {
       version v ("1+0");
       assert (v.string () == "1+0");
-      assert (v.canonical_upstream ().empty ());
+      assert (v.canonical_upstream.empty ());
       assert (test_constructor (v));
     }
 
     {
       version v ("0+A-1");
       assert (v.string () == "A-1");
-      assert (v.canonical_upstream () == "a");
+      assert (v.canonical_upstream == "a");
       assert (test_constructor (v));
     }
 
     {
       version v ("10+B-0");
       assert (v.string () == "10+B");
-      assert (v.canonical_upstream () == "b");
+      assert (v.canonical_upstream == "b");
       assert (test_constructor (v));
     }
 
     {
       version v ("3+1A.31.0.4.0-7");
       assert (v.string () == "3+1A.31.0.4.0-7");
-      assert (v.canonical_upstream () == "1a.00000031.00000000.00000004");
+      assert (v.canonical_upstream == "1a.00000031.00000000.00000004");
       assert (test_constructor (v));
     }