optimized RBTree: avoid unnecessary virtual RBTree and RBEntry implementation classes

 * whenever the implementation class was not exactly RBEntryType,
   it had to have a virtual destructor since RBTree cared about
   cleanup and had to delete its members.
 * Since it does not allocate them, it is consistent to remove RBTree::clear().
   The destructor now only checks that subclasses have cleaned up.
   Implementing cleanup in the subclasses is trivial.
 * Consequently, RBEntryString no longer has to be virtual.
   HelpIndex and GotoTables are completely non-virtual now
   which saves memory (and a bit of cleanup speed).
   For QRegister, not much changes, though.

7 files changed, 42 insertions, 26 deletions

diff --git a/src/goto.h b/src/goto.h
index f2064d1..5d29e8b 100644
--- a/src/goto.h
+++ b/src/goto.h
@@ -77,6 +77,11 @@ class GotoTable : private RBTreeString {
 public:
 	GotoTable(bool _undo = true) : must_undo(_undo) {}
 
+	~GotoTable()
+	{
+		clear();
+	}
+
 	gint remove(const gchar *name);
 	gint find(const gchar *name);
 
@@ -91,7 +96,10 @@ public:
 	inline void
 	clear(void)
 	{
-		RBTreeString::clear();
+		Label *cur;
+
+		while ((cur = (Label *)min()))
+			delete (Label *)RBTreeString::remove(cur);
 	}
 
 	inline gchar *
diff --git a/src/help.cpp b/src/help.cpp
index 542a11f..3b82c4c 100644
--- a/src/help.cpp
+++ b/src/help.cpp
@@ -51,7 +51,7 @@ HelpIndex::load(void)
 	GDir *women_dir;
 	const gchar *basename;
 
-	if (G_UNLIKELY(min() != NULL))
+	if (G_LIKELY(min() != NULL))
 		/* already loaded */
 		return;
 
diff --git a/src/help.h b/src/help.h
index 92115ab..55fb2ee 100644
--- a/src/help.h
+++ b/src/help.h
@@ -47,6 +47,14 @@ public:
 		}
 	};
 
+	~HelpIndex()
+	{
+		Topic *cur;
+
+		while ((cur = (Topic *)min()))
+			delete (Topic *)remove(cur);
+	}
+
 	void load(void);
 
 	Topic *find(const gchar *name);
diff --git a/src/qregisters.cpp b/src/qregisters.cpp
index f6c89a8..821cbc2 100644
--- a/src/qregisters.cpp
+++ b/src/qregisters.cpp
@@ -841,7 +841,7 @@ QRegisterTable::update_environ(void)
 /**
  * Free resources associated with table.
  *
- * This is similar to RBTree::clear() but
+ * This is similar to the destructor but
  * has the advantage that we can check whether some
  * register is currently edited.
  * Since this is not a destructor, we can throw
@@ -859,8 +859,7 @@ QRegisterTable::clear(void)
 			throw Error("Currently edited Q-Register \"%s\" "
 			            "cannot be discarded", cur->name);
 
-		remove(cur);
-		delete cur;
+		delete (QRegister *)remove(cur);
 	}
 }
 
diff --git a/src/qregisters.h b/src/qregisters.h
index 1d0962c..ef06458 100644
--- a/src/qregisters.h
+++ b/src/qregisters.h
@@ -146,6 +146,8 @@ public:
 	QRegister(const gchar *name)
 		 : RBTreeString::RBEntryOwnString(name) {}
 
+	virtual ~QRegister() {}
+
 	virtual void edit(void);
 	virtual void undo_edit(void);
 
@@ -308,6 +310,14 @@ class QRegisterTable : private RBTreeString {
 public:
 	QRegisterTable(bool _undo = true);
 
+	~QRegisterTable()
+	{
+		QRegister *cur;
+
+		while ((cur = (QRegister *)min()))
+			delete (QRegister *)remove(cur);
+	}
+
 	inline void
 	undo_remove(QRegister *reg)
 	{
diff --git a/src/rbtree.cpp b/src/rbtree.cpp
index 5b1a4e1..79c63d6 100644
--- a/src/rbtree.cpp
+++ b/src/rbtree.cpp
@@ -48,7 +48,7 @@ auto_complete(const gchar *key, gchar completed, gsize restrict_len)
 
 	for (RBEntryString *cur = nfind(key);
 	     cur && !StringNCmp(cur->key, key, key_len);
-	     cur = (RBEntryString *)cur->next()) {
+	     cur = cur->next()) {
 		if (restrict_len && strlen(cur->key) != restrict_len)
 			continue;
 
@@ -68,7 +68,7 @@ auto_complete(const gchar *key, gchar completed, gsize restrict_len)
 	if (!insert && prefixed_entries > 1) {
 		for (RBEntryString *cur = first;
 		     cur && !StringNCmp(cur->key, key, key_len);
-		     cur = (RBEntryString *)cur->next()) {
+		     cur = cur->next()) {
 			if (restrict_len && strlen(cur->key) != restrict_len)
 				continue;
 
diff --git a/src/rbtree.h b/src/rbtree.h
index 76b2141..7730fc5 100644
--- a/src/rbtree.h
+++ b/src/rbtree.h
@@ -34,15 +34,15 @@ public:
 	public:
 		RB_ENTRY(RBEntry) nodes;
 
-		inline RBEntry *
+		inline RBEntryType *
 		next(void)
 		{
-			return RBTree::Tree_RB_NEXT(this);
+			return (RBEntryType *)RBTree::Tree_RB_NEXT(this);
 		}
-		inline RBEntry *
+		inline RBEntryType *
 		prev(void)
 		{
-			return RBTree::Tree_RB_PREV(this);
+			return (RBEntryType *)RBTree::Tree_RB_PREV(this);
 		}
 	};
 
@@ -66,10 +66,14 @@ public:
 	{
 		RB_INIT(&head);
 	}
-	virtual
 	~RBTree()
 	{
-		clear();
+		/*
+		 * Keeping the clean up out of this wrapper class
+		 * means we can avoid declaring EBEntry implementations
+		 * virtual.
+		 */
+		g_assert(min() == NULL);
 	}
 
 	inline RBEntryType *
@@ -108,17 +112,6 @@ public:
 	{
 		return (RBEntryType *)RB_MAX(Tree, &head);
 	}
-
-	inline void
-	clear(void)
-	{
-		RBEntryType *cur;
-
-		while ((cur = min())) {
-			remove(cur);
-			delete cur;
-		}
-	}
 };
 
 typedef gint (*StringCmpFunc)(const gchar *str1, const gchar *str2);
@@ -138,8 +131,6 @@ public:
 
 	RBEntryStringT(gchar *_key) : key(_key) {}
 
-	virtual ~RBEntryStringT() {}
-
 	inline gint
 	compare(RBEntryStringT &other)
 	{