[PATCH] D25645: [ADT] Add CachedHashString.

[David Blaikie via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/ADT/CachedHashString.h:117
+  CachedHashString &operator=(const CachedHashString &Other) {
+    if (!isEmptyOrTombstone())
+      delete[] P;
----------------
timshen wrote:
> How do you feel about this:
>   if (this != Other) {
>     ~CachedHashString();
>     new (this) CachedHashString(Other);
>   }
>   return *this;
> 
> Likewise for the move assign.
You can potentially just use the unified copy/move op roughly:

  foo &operator=(foo f) {
    swap(*this, f);
  }

(you can actually implement the assignment here rather than use a custom swap - then rely on std::swap to be cheap enough - or the other way around, whichever's better/worth it)

& then you do need to implement copy and move ctors, but you've already got those.


https://reviews.llvm.org/D25645