ADT: Guarantee transferNodesFromList is only called on transfers

Guarantee that ilist_traits<T>::transferNodesFromList is only called
when nodes are actually changing lists.

I also moved all the callbacks to occur *first*, before the operation.
This is the only choice for iplist<T>::merge, so we might as well be
consistent.  I expect this to have no effect in practice, although it
simplifies the logic in both iplist<T>::transfer and iplist<T>::insert.

llvm-svn: 280122

llvm/include/llvm/ADT/ilist.h

@@ -43,9 +43,15 @@ template <typename NodeTy> struct ilist_node_traits {
 
   void addNodeToList(NodeTy *) {}
   void removeNodeFromList(NodeTy *) {}
-  void transferNodesFromList(ilist_node_traits & /*SrcTraits*/,
+
+  /// Callback before transferring nodes to this list.
+  ///
+  /// \pre \c this!=&OldList
+  void transferNodesFromList(ilist_node_traits &OldList,
                              ilist_iterator<NodeTy> /*first*/,
-                             ilist_iterator<NodeTy> /*last*/) {}
+                             ilist_iterator<NodeTy> /*last*/) {
+    (void)OldList;
+  }
 };
 
 /// Default template traits for intrusive list.
@@ -165,9 +171,8 @@ public:
   }
 
   iterator insert(iterator where, NodeTy *New) {
-    auto I = base_list_type::insert(where, *New);
-    this->addNodeToList(New);  // Notify traits that we added a node...
-    return I;
+    this->addNodeToList(New); // Notify traits that we added a node...
+    return base_list_type::insert(where, *New);
   }
 
   iterator insert(iterator where, const NodeTy &New) {
@@ -182,9 +187,9 @@ public:
   }
 
   NodeTy *remove(iterator &IT) {
-    NodeTy *Node = &*IT;
-    base_list_type::erase(IT++);
-    this->removeNodeFromList(Node);  // Notify traits that we removed a node...
+    NodeTy *Node = &*IT++;
+    this->removeNodeFromList(Node); // Notify traits that we removed a node...
+    base_list_type::remove(*Node);
     return Node;
   }
 
@@ -220,11 +225,10 @@ private:
     if (position == last)
       return;
 
-    base_list_type::splice(position, L2, first, last);
+    if (this != &L2) // Notify traits we moved the nodes...
+      this->transferNodesFromList(L2, first, last);
 
-    // Callback.  Note that the nodes have moved from before-last to
-    // before-position.
-    this->transferNodesFromList(L2, first, position);
+    base_list_type::splice(position, L2, first, last);
   }
 
 public:

llvm/lib/CodeGen/MachineBasicBlock.cpp

@@ -122,9 +122,8 @@ transferNodesFromList(ilist_traits<MachineInstr> &FromList,
                       ilist_iterator<MachineInstr> Last) {
   assert(Parent->getParent() == FromList.Parent->getParent() &&
         "MachineInstr parent mismatch!");
-
-  // Splice within the same MBB -> no change.
-  if (Parent == FromList.Parent) return;
+  assert(this != &FromList && "Called without a real transfer...");
+  assert(Parent != FromList.Parent && "Two lists have the same parent?");
 
   // If splicing between two blocks within the same function, just update the
   // parent pointers.

llvm/lib/IR/SymbolTableListTraitsImpl.h

@@ -85,7 +85,7 @@ void SymbolTableListTraits<ValueSubClass>::transferNodesFromList(
     ilist_iterator<ValueSubClass> last) {
   // We only have to do work here if transferring instructions between BBs
   ItemParentClass *NewIP = getListOwner(), *OldIP = L2.getListOwner();
-  if (NewIP == OldIP) return;  // No work to do at all...
+  assert(NewIP != OldIP && "Expected different list owners");
 
   // We only have to update symbol table entries if we are transferring the
   // instructions to a different symtab object...