commit 2c14af9b75dab883e728868b6215ae6a743f5c29
author Bryan Henry <bryanhenry@google.com> Thu Jan 10 18:32:52 2019 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Sat Jan 12 01:52:20 2019 +0000
tree 22d9002fcba395dc73753ffa63108aebb0d6f1e1
parent ece59c3dc4de548328523d2e88843e21f31ce321

[fidl][cpp] Unify StringPtr nullity semantics

Previously, is_null() and operator bool() disagreed about the nullity of
a non-empty string with the is_null bit set.

This inconsistency was introduced in fxr/136820 and was previously fixed
for VectorPtr in fxr/148924 but never fixed for StringPtr.

Test: fx run-host-tests fidl_cpp_host_unittests
Change-Id: Ife7119c100bd171cb6b097bed24c96a3bb2cec24

public/lib/fidl/cpp/string.cc

diff --git a/public/lib/fidl/cpp/string.cc b/public/lib/fidl/cpp/string.cc
index d714ef9..e31a93d 100644
--- a/public/lib/fidl/cpp/string.cc
+++ b/public/lib/fidl/cpp/string.cc
@@ -10,28 +10,30 @@
 
 namespace fidl {
 
-StringPtr::StringPtr() : is_null_(true) {}
+StringPtr::StringPtr() : is_null_if_empty_(true) {}
 
 StringPtr::StringPtr(const StringPtr& other) = default;
 
-StringPtr::StringPtr(std::string str) : str_(std::move(str)), is_null_(false) {}
+StringPtr::StringPtr(std::string str)
+    : str_(std::move(str)), is_null_if_empty_(false) {}
 
 StringPtr::StringPtr(const char* str)
-    : str_(str ? std::string(str) : std::string()), is_null_(!str) {}
+    : str_(str ? std::string(str) : std::string()), is_null_if_empty_(!str) {}
 
 StringPtr::StringPtr(const char* str, size_t length)
-    : str_(str ? std::string(str, length) : std::string()), is_null_(!str) {}
+    : str_(str ? std::string(str, length) : std::string()),
+      is_null_if_empty_(!str) {}
 
 StringPtr::~StringPtr() = default;
 
 StringPtr::StringPtr(StringPtr&& other)
-    : str_(std::move(other.str_)), is_null_(other.is_null_) {}
+    : str_(std::move(other.str_)), is_null_if_empty_(other.is_null_if_empty_) {}
 
 StringPtr& StringPtr::operator=(const StringPtr& other) = default;
 
 StringPtr& StringPtr::operator=(StringPtr&& other) {
   str_ = std::move(other.str_);
-  is_null_ = other.is_null_;
+  is_null_if_empty_ = other.is_null_if_empty_;
   return *this;
 }
 

public/lib/fidl/cpp/string.h

diff --git a/public/lib/fidl/cpp/string.h b/public/lib/fidl/cpp/string.h
index e2e95ed..aa147a1 100644
--- a/public/lib/fidl/cpp/string.h
+++ b/public/lib/fidl/cpp/string.h
@@ -46,28 +46,28 @@
   // After this method returns, the StringPtr is non-null.
   void reset(std::string str) {
     str_ = std::move(str);
-    is_null_ = false;
+    is_null_if_empty_ = false;
   }
 
   // Causes this StringPtr to become null.
   void reset() {
     str_.clear();
-    is_null_ = true;
+    is_null_if_empty_ = true;
   }
 
   void swap(StringPtr& other) {
     using std::swap;
     swap(str_, other.str_);
-    swap(is_null_, other.is_null_);
+    swap(is_null_if_empty_, other.is_null_if_empty_);
   }
 
   // Whether this StringPtr is null.
   //
   // The null state is separate from the empty state.
-  bool is_null() const { return is_null_ && str_.empty(); }
+  bool is_null() const { return is_null_if_empty_ && str_.empty(); }
 
   // Tests as true if non-null, false if null.
-  explicit operator bool() const { return !is_null_; }
+  explicit operator bool() const { return !is_null(); }
 
   // Provides access to the underlying std::string.
   std::string* operator->() { return &str_; }
@@ -86,7 +86,7 @@
 
  private:
   std::string str_;
-  bool is_null_;
+  bool is_null_if_empty_;
 };
 
 inline bool operator==(const StringPtr& a, const StringPtr& b) {

public/lib/fidl/cpp/string_unittest.cc

diff --git a/public/lib/fidl/cpp/string_unittest.cc b/public/lib/fidl/cpp/string_unittest.cc
index 90a7783..dd8359f 100644
--- a/public/lib/fidl/cpp/string_unittest.cc
+++ b/public/lib/fidl/cpp/string_unittest.cc
@@ -14,6 +14,7 @@
   EXPECT_FALSE(string);
   string->append("abc");
   EXPECT_FALSE(string.is_null());
+  EXPECT_TRUE(string);
 
   string.reset("hello, world");
   EXPECT_FALSE(string.is_null());