cmListFileCache: Refactor cmListFileBacktrace internals

Replace use of raw pointers and explicit reference counting with
`std::shared_ptr<>`.  Use a discriminated union to store either the
bottom level or a call/file context in each heap-allocated entry.
This avoids storing a copy of the bottom in every `cmListFileBacktrace`
instance and shrinks the structure to a single `shared_ptr`.

1 files changed, 75 insertions, 89 deletions

diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx
index b468257..e465e1a 100644
--- a/Source/cmListFileCache.cxx
+++ b/Source/cmListFileCache.cxx
@@ -11,6 +11,7 @@
 
 #include <algorithm>
 #include <assert.h>
+#include <memory>
 #include <sstream>
 
 cmCommandContext::cmCommandName& cmCommandContext::cmCommandName::operator=(
@@ -285,91 +286,66 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
   return true;
 }
 
-struct cmListFileBacktrace::Entry : public cmListFileContext
+// We hold either the bottom scope of a directory or a call/file context.
+// Discriminate these cases via the parent pointer.
+struct cmListFileBacktrace::Entry
 {
-  Entry(cmListFileContext const& lfc, Entry* up)
-    : cmListFileContext(lfc)
-    , Up(up)
-    , RefCount(0)
+  Entry(cmStateSnapshot bottom)
+    : Bottom(bottom)
   {
-    if (this->Up) {
-      this->Up->Ref();
-    }
   }
-  ~Entry()
+
+  Entry(std::shared_ptr<Entry const> parent, cmListFileContext lfc)
+    : Context(std::move(lfc))
+    , Parent(std::move(parent))
   {
-    if (this->Up) {
-      this->Up->Unref();
-    }
   }
-  void Ref() { ++this->RefCount; }
-  void Unref()
+
+  ~Entry()
   {
-    if (--this->RefCount == 0) {
-      delete this;
+    if (this->Parent) {
+      this->Context.~cmListFileContext();
+    } else {
+      this->Bottom.~cmStateSnapshot();
     }
   }
-  Entry* Up;
-  unsigned int RefCount;
-};
 
-cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& bottom,
-                                         Entry* up,
-                                         cmListFileContext const& lfc)
-  : Bottom(bottom)
-  , Cur(new Entry(lfc, up))
-{
-  assert(this->Bottom.IsValid());
-  this->Cur->Ref();
-}
-
-cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& bottom,
-                                         Entry* cur)
-  : Bottom(bottom)
-  , Cur(cur)
-{
-  if (this->Cur) {
-    assert(this->Bottom.IsValid());
-    this->Cur->Ref();
-  }
-}
+  bool IsBottom() const { return !this->Parent; }
 
-cmListFileBacktrace::cmListFileBacktrace()
-  : Bottom()
-  , Cur(nullptr)
-{
-}
+  union
+  {
+    cmStateSnapshot Bottom;
+    cmListFileContext Context;
+  };
+  std::shared_ptr<Entry const> Parent;
+};
 
 cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& snapshot)
-  : Bottom(snapshot.GetCallStackBottom())
-  , Cur(nullptr)
+  : TopEntry(std::make_shared<Entry const>(snapshot.GetCallStackBottom()))
 {
 }
 
-cmListFileBacktrace::cmListFileBacktrace(cmListFileBacktrace const& r)
-  : Bottom(r.Bottom)
-  , Cur(r.Cur)
+cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> parent,
+                                         cmListFileContext const& lfc)
+  : TopEntry(std::make_shared<Entry const>(std::move(parent), lfc))
 {
-  if (this->Cur) {
-    assert(this->Bottom.IsValid());
-    this->Cur->Ref();
-  }
 }
 
-cmListFileBacktrace& cmListFileBacktrace::operator=(
-  cmListFileBacktrace const& r)
+cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> top)
+  : TopEntry(std::move(top))
 {
-  cmListFileBacktrace tmp(r);
-  std::swap(this->Cur, tmp.Cur);
-  std::swap(this->Bottom, tmp.Bottom);
-  return *this;
 }
 
-cmListFileBacktrace::~cmListFileBacktrace()
+cmStateSnapshot cmListFileBacktrace::GetBottom() const
 {
-  if (this->Cur) {
-    this->Cur->Unref();
+  cmStateSnapshot bottom;
+  if (Entry const* cur = this->TopEntry.get()) {
+    while (Entry const* parent = cur->Parent.get()) {
+      cur = parent;
+    }
+    bottom = cur->Bottom;
   }
+  return bottom;
 }
 
 cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const
@@ -380,54 +356,61 @@ cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const
   // skipped during call stack printing.
   cmListFileContext lfc;
   lfc.FilePath = file;
-  return cmListFileBacktrace(this->Bottom, this->Cur, lfc);
+  return this->Push(lfc);
 }
 
 cmListFileBacktrace cmListFileBacktrace::Push(
   cmListFileContext const& lfc) const
 {
-  return cmListFileBacktrace(this->Bottom, this->Cur, lfc);
+  assert(this->TopEntry);
+  assert(!this->TopEntry->IsBottom() || this->TopEntry->Bottom.IsValid());
+  return cmListFileBacktrace(this->TopEntry, lfc);
 }
 
 cmListFileBacktrace cmListFileBacktrace::Pop() const
 {
-  assert(this->Cur);
-  return cmListFileBacktrace(this->Bottom, this->Cur->Up);
+  assert(this->TopEntry);
+  assert(!this->TopEntry->IsBottom());
+  return cmListFileBacktrace(this->TopEntry->Parent);
 }
 
 cmListFileContext const& cmListFileBacktrace::Top() const
 {
-  if (this->Cur) {
-    return *this->Cur;
-  }
-  static cmListFileContext const empty;
-  return empty;
+  assert(this->TopEntry);
+  return this->TopEntry->Context;
 }
 
 void cmListFileBacktrace::PrintTitle(std::ostream& out) const
 {
-  if (!this->Cur) {
+  // The title exists only if we have a call on top of the bottom.
+  if (!this->TopEntry || this->TopEntry->IsBottom()) {
     return;
   }
-  cmOutputConverter converter(this->Bottom);
-  cmListFileContext lfc = *this->Cur;
-  if (!this->Bottom.GetState()->GetIsInTryCompile()) {
+  cmListFileContext lfc = this->TopEntry->Context;
+  cmStateSnapshot bottom = this->GetBottom();
+  cmOutputConverter converter(bottom);
+  if (!bottom.GetState()->GetIsInTryCompile()) {
     lfc.FilePath = converter.ConvertToRelativePath(
-      this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
+      bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
   }
   out << (lfc.Line ? " at " : " in ") << lfc;
 }
 
 void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
 {
-  if (!this->Cur || !this->Cur->Up) {
+  // The call stack exists only if we have at least two calls on top
+  // of the bottom.
+  if (!this->TopEntry || this->TopEntry->IsBottom() ||
+      this->TopEntry->Parent->IsBottom()) {
     return;
   }
 
   bool first = true;
-  cmOutputConverter converter(this->Bottom);
-  for (Entry* i = this->Cur->Up; i; i = i->Up) {
-    if (i->Name.empty()) {
+  cmStateSnapshot bottom = this->GetBottom();
+  cmOutputConverter converter(bottom);
+  for (Entry const* cur = this->TopEntry->Parent.get(); !cur->IsBottom();
+       cur = cur->Parent.get()) {
+    if (cur->Context.Name.empty()) {
       // Skip this whole-file scope.  When we get here we already will
       // have printed a more-specific context within the file.
       continue;
@@ -436,10 +419,10 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
       first = false;
       out << "Call Stack (most recent call first):\n";
     }
-    cmListFileContext lfc = *i;
-    if (!this->Bottom.GetState()->GetIsInTryCompile()) {
+    cmListFileContext lfc = cur->Context;
+    if (!bottom.GetState()->GetIsInTryCompile()) {
       lfc.FilePath = converter.ConvertToRelativePath(
-        this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
+        bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
     }
     out << "  " << lfc << "\n";
   }
@@ -448,16 +431,19 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
 size_t cmListFileBacktrace::Depth() const
 {
   size_t depth = 0;
-  if (this->Cur == nullptr) {
-    return 0;
-  }
-
-  for (Entry* i = this->Cur->Up; i; i = i->Up) {
-    depth++;
+  if (Entry const* cur = this->TopEntry.get()) {
+    for (; !cur->IsBottom(); cur = cur->Parent.get()) {
+      ++depth;
+    }
   }
   return depth;
 }
 
+bool cmListFileBacktrace::Empty() const
+{
+  return !this->TopEntry || this->TopEntry->IsBottom();
+}
+
 std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc)
 {
   os << lfc.FilePath;