Fix new lints

Author: gspencergoog

fml/memory/ref_counted_unittest.cc

@@ -227,7 +227,7 @@ TEST(RefCountedTest, NullAssignmentToNull) {
   // No-op null assignment using move constructor.
   r1 = std::move(r2);
   EXPECT_TRUE(r1.get() == nullptr);
-  EXPECT_TRUE(r2.get() == nullptr);
+  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r2);
 
@@ -272,7 +272,7 @@ TEST(RefCountedTest, NonNullAssignmentToNull) {
     RefPtr<MyClass> r2;
     // Move assignment (to null ref pointer).
     r2 = std::move(r1);
-    EXPECT_TRUE(r1.get() == nullptr);
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
     EXPECT_EQ(created, r2.get());
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);
@@ -336,7 +336,7 @@ TEST(RefCountedTest, NullAssignmentToNonNull) {
   // Null assignment using move constructor.
   r1 = std::move(r2);
   EXPECT_TRUE(r1.get() == nullptr);
-  EXPECT_TRUE(r2.get() == nullptr);
+  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r2);
   EXPECT_TRUE(was_destroyed);
@@ -389,7 +389,7 @@ TEST(RefCountedTest, NonNullAssignmentToNonNull) {
     RefPtr<MyClass> r2(MakeRefCounted<MyClass>(nullptr, &was_destroyed2));
     // Move assignment (to non-null ref pointer).
     r2 = std::move(r1);
-    EXPECT_TRUE(r1.get() == nullptr);
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
     EXPECT_FALSE(r2.get() == nullptr);
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);
@@ -598,13 +598,14 @@ TEST(RefCountedTest, PublicCtorAndDtor) {
 TEST(RefCountedTest, DebugChecks) {
   {
     MyPublicClass* p = new MyPublicClass();
-    EXPECT_DEATH_IF_SUPPORTED(delete p, "!adoption_required_");
+    EXPECT_DEATH_IF_SUPPORTED(  // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks)
+        delete p, "!adoption_required_");
   }
 
   {
     MyPublicClass* p = new MyPublicClass();
-    EXPECT_DEATH_IF_SUPPORTED(RefPtr<MyPublicClass> r(p),
-                              "!adoption_required_");
+    EXPECT_DEATH_IF_SUPPORTED(  // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks)
+        RefPtr<MyPublicClass> r(p), "!adoption_required_");
   }
 
   {

fml/memory/ref_ptr.h

@@ -66,42 +66,48 @@ template <typename T>
 class RefPtr final {
  public:
   RefPtr() : ptr_(nullptr) {}
-  RefPtr(std::nullptr_t) : ptr_(nullptr) {}
+  RefPtr(std::nullptr_t)
+      : ptr_(nullptr) {}  // NOLINT(google-explicit-constructor)
 
   // Explicit constructor from a plain pointer (to an object that must have
   // already been adopted). (Note that in |T::T()|, references to |this| cannot
   // be taken, since the object being constructed will not have been adopted
   // yet.)
   template <typename U>
   explicit RefPtr(U* p) : ptr_(p) {
-    if (ptr_)
+    if (ptr_) {
       ptr_->AddRef();
+    }
   }
 
   // Copy constructor.
   RefPtr(const RefPtr<T>& r) : ptr_(r.ptr_) {
-    if (ptr_)
+    if (ptr_) {
       ptr_->AddRef();
+    }
   }
 
   template <typename U>
-  RefPtr(const RefPtr<U>& r) : ptr_(r.ptr_) {
-    if (ptr_)
+  RefPtr(const RefPtr<U>& r)
+      : ptr_(r.ptr_) {  // NOLINT(google-explicit-constructor)
+    if (ptr_) {
       ptr_->AddRef();
+    }
   }
 
   // Move constructor.
   RefPtr(RefPtr<T>&& r) : ptr_(r.ptr_) { r.ptr_ = nullptr; }
 
   template <typename U>
-  RefPtr(RefPtr<U>&& r) : ptr_(r.ptr_) {
+  RefPtr(RefPtr<U>&& r) : ptr_(r.ptr_) {  // NOLINT(google-explicit-constructor)
     r.ptr_ = nullptr;
   }
 
   // Destructor.
   ~RefPtr() {
-    if (ptr_)
+    if (ptr_) {
       ptr_->Release();
+    }
   }
 
   T* get() const { return ptr_; }
@@ -118,25 +124,34 @@ class RefPtr final {
 
   // Copy assignment.
   RefPtr<T>& operator=(const RefPtr<T>& r) {
-    // Call |AddRef()| first so self-assignments work.
-    if (r.ptr_)
+    // Handle self-assignment.
+    if (r.ptr_ == ptr_) {
+      return *this;
+    }
+    if (r.ptr_) {
       r.ptr_->AddRef();
+    }
     T* old_ptr = ptr_;
     ptr_ = r.ptr_;
-    if (old_ptr)
+    if (old_ptr) {
       old_ptr->Release();
+    }
     return *this;
   }
 
   template <typename U>
   RefPtr<T>& operator=(const RefPtr<U>& r) {
-    // Call |AddRef()| first so self-assignments work.
-    if (r.ptr_)
+    if (reinterpret_cast<T*>(r.ptr_) == ptr_) {
+      return *this;
+    }
+    if (r.ptr_) {
       r.ptr_->AddRef();
+    }
     T* old_ptr = ptr_;
     ptr_ = r.ptr_;
-    if (old_ptr)
+    if (old_ptr) {
       old_ptr->Release();
+    }
     return *this;
   }