Return iterator from Instruction::eraseFromParent.

[• dberlin]

Summary

This is necessary in order to make removal while using reverse iterators work.

(See http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-March/084122.html)

Updates to other eraseFromParent's to come in later patches.

[• dberlin]

Mark the function i moved to be inline (Darwin apparently does not care for some reason).

Also happy to move it into a .cpp file if we like, just trying to replicate the same single-module inlining availability
that existed before

[chandlerc]

Looks good.

include/llvm/IR/BasicBlock.h

 namespace llvm {
 
 class CallInst;
 class LandingPadInst;
 class TerminatorInst;
 class LLVMContext;
 class BlockAddress;
 
-template<> struct ilist_traits<Instruction>
-  : public SymbolTableListTraits<Instruction, BasicBlock> {
-
-  /// \brief Return a node that marks the end of a list.
-  ///
-  /// The sentinel is relative to this instance, so we use a non-static
-  /// method.
-  Instruction *createSentinel() const {
-    // Since i(p)lists always publicly derive from their corresponding traits,
-    // placing a data member in this class will augment the i(p)list.  But since
-    // the NodeTy is expected to be publicly derive from ilist_node<NodeTy>,
-    // there is a legal viable downcast from it to NodeTy. We use this trick to
-    // superimpose an i(p)list with a "ghostly" NodeTy, which becomes the
-    // sentinel. Dereferencing the sentinel is forbidden (save the
-    // ilist_node<NodeTy>), so no one will ever notice the superposition.
-    return static_cast<Instruction*>(&Sentinel);
-  }
-  static void destroySentinel(Instruction*) {}
-
-  Instruction *provideInitialHead() const { return createSentinel(); }
-  Instruction *ensureHead(Instruction*) const { return createSentinel(); }
-  static void noteHead(Instruction*, Instruction*) {}
-private:
-  mutable ilist_half_node<Instruction> Sentinel;
-};
 
 /// \brief LLVM Basic Block Representation
 ///
 /// This represents a single basic block in LLVM. A basic block is simply a
 /// container of instructions that execute sequentially. Basic blocks are Values
 /// because they are referenced by instructions such as branches and switch
 /// tables. The type of a BasicBlock is "Type::LabelTy" because the basic block
 /// represents a label to which a branch can jump.

include/llvm/IR/Instruction.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_INSTRUCTION_H
 #define LLVM_IR_INSTRUCTION_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/IR/DebugLoc.h"
+#include "llvm/IR/SymbolTableListTraits.h"
 #include "llvm/IR/User.h"
 
 namespace llvm {
 
 class FastMathFlags;
 class LLVMContext;
 class MDNode;
+class BasicBlock;
 struct AAMDNodes;
 
-template<typename ValueSubClass, typename ItemParentClass>
-  class SymbolTableListTraits;
+template <>
+struct ilist_traits<Instruction>
+    : public SymbolTableListTraits<Instruction, BasicBlock> {
+
+  /// \brief Return a node that marks the end of a list.
+  ///
+  /// The sentinel is relative to this instance, so we use a non-static
+  /// method.
+  Instruction *createSentinel() const;
+  static void destroySentinel(Instruction *) {}
+
+  Instruction *provideInitialHead() const { return createSentinel(); }
+  Instruction *ensureHead(Instruction *) const { return createSentinel(); }
+  static void noteHead(Instruction *, Instruction *) {}
+
+private:
+  mutable ilist_half_node<Instruction> Sentinel;
+};
 
 class Instruction : public User, public ilist_node<Instruction> {
   void operator=(const Instruction &) = delete;
   Instruction(const Instruction &) = delete;
 
   BasicBlock *Parent;
   DebugLoc DbgLoc;                         // 'dbg' Metadata cache.
 
public:
   /// removeFromParent - This method unlinks 'this' from the containing basic
   /// block, but does not delete it.
   ///
   void removeFromParent();
 
   /// eraseFromParent - This method unlinks 'this' from the containing basic
   /// block and deletes it.
   ///
-  void eraseFromParent();
+  /// \returns an iterator pointing to the element after the erased one
+  iplist<Instruction>::iterator eraseFromParent();
 
   /// insertBefore - Insert an unlinked instructions into a basic block
   /// immediately before the specified instruction.
   void insertBefore(Instruction *InsertPos);
 
   /// insertAfter - Insert an unlinked instructions into a basic block
   /// immediately after the specified instruction.
   void insertAfter(Instruction *InsertPos);
protected:
   Instruction(Type *Ty, unsigned iType, Use *Ops, unsigned NumOps,
               Instruction *InsertBefore = nullptr);
   Instruction(Type *Ty, unsigned iType, Use *Ops, unsigned NumOps,
               BasicBlock *InsertAtEnd);
   virtual Instruction *clone_impl() const = 0;
 
 };
 
+Instruction *ilist_traits<Instruction>::createSentinel() const {
+  // Since i(p)lists always publicly derive from their corresponding traits,
+  // placing a data member in this class will augment the i(p)list.  But since
+  // the NodeTy is expected to be publicly derive from ilist_node<NodeTy>,
+  // there is a legal viable downcast from it to NodeTy. We use this trick to
+  // superimpose an i(p)list with a "ghostly" NodeTy, which becomes the
+  // sentinel. Dereferencing the sentinel is forbidden (save the
+  // ilist_node<NodeTy>), so no one will ever notice the superposition.
+  return static_cast<Instruction *>(&Sentinel);
+}
+
 // Instruction* is only 4-byte aligned.
 template<>
 class PointerLikeTypeTraits<Instruction*> {
   typedef Instruction* PT;
 public:
   static inline void *getAsVoidPointer(PT P) { return P; }
   static inline PT getFromVoidPointer(void *P) {
     return static_cast<PT>(P);
   }
   enum { NumLowBitsAvailable = 2 };
 };
 
 } // End llvm namespace
 
 #endif

lib/IR/Instruction.cpp

 const Module *Instruction::getModule() const {
   return getParent()->getModule();
 }
 
 void Instruction::removeFromParent() {
   getParent()->getInstList().remove(this);
 }
 
-void Instruction::eraseFromParent() {
-  getParent()->getInstList().erase(this);
+iplist<Instruction>::iterator Instruction::eraseFromParent() {
+  return getParent()->getInstList().erase(this);
 }
 
 /// insertBefore - Insert an unlinked instructions into a basic block
 /// immediately before the specified instruction.
 void Instruction::insertBefore(Instruction *InsertPos) {
   InsertPos->getParent()->getInstList().insert(InsertPos, this);
 }