Second attemmpt at fixing critical bug in Node::unref()

include/bolt/CST.hpp

@@ -197,12 +197,7 @@ namespace bolt {
       ++RefCount;
     }
 
-    inline void unref() {
+    void unref();
-      --RefCount;
-      if (RefCount == 0) {
-        delete this;
-      }
-    }
 
     void setParents();
 
@@ -249,7 +244,7 @@ namespace bolt {
 
     virtual Scope* getScope();
 
-    virtual ~Node();
+    virtual ~Node() {}
 
   };
 

src/CST.cc

@@ -289,19 +289,26 @@ namespace bolt {
 
   }
 
-  Node::~Node() {
+  void Node::unref() {
 
-    struct UnrefVisitor : public CSTVisitor<UnrefVisitor> {
+    --RefCount;
 
-      void visit(Node* N) {
+    if (RefCount == 0) {
-        visitEachChild(N);
-        N->unref();
-      }
 
-    };
+      // You may be wondering why we aren't unreffing the children in the
+      // destructor. This is due to a behaviour in Clang where a top-level
+      // destructor ~Node() wont get access to the fields in derived classes
+      // because they may already have been destroyed.
+      struct UnrefVisitor : public CSTVisitor<UnrefVisitor> {
+        void visit(Node* N) {
+          N->unref();
+        }
+      };
+      UnrefVisitor V;
+      V.visitEachChild(this);
 
-    UnrefVisitor V;
+      delete this;
-    V.visitEachChild(this);
+    }
 
   }