[mlir] Allow SymbolTable to update existing symbols

[tpopp]

Summary

Previous behavior would fail if inserting an operation that already
existed. Now SymbolTable::insert can also be used as a way to make a
symbol's name unique even after insertion.

Further TODOs have been left over naming and consistent behavior
considerations.

[tpopp]

Correct typo and add comment

[rriddle]

Nice!

mlir/lib/IR/SymbolTable.cpp

155

Can you update the doc here and in the header? It should probably say that it only inserts into the symbol table operation if it isn't already a child.

158

All of this can be wrapped into the if given that it is only useful when inserting the operation.

mlir/lib/IR/SymbolTable.cpp

void SymbolTable::erase(Operation *symbol) {
 
   auto it = symbolTable.find(*name);
   if (it != symbolTable.end() && it->second == symbol) {
     symbolTable.erase(it);
     symbol->erase();
   }
 }
 
+// TODO: Consider if this should be renamed to something like insertOrUpdate
 /// Insert a new symbol into the table and associated operation, and rename it
 /// as necessary to avoid collisions.
 void SymbolTable::insert(Operation *symbol, Block::iterator insertPt) {
   auto &body = symbolTableOp->getRegion(0).front();
   if (insertPt == Block::iterator() || insertPt == body.end())
     insertPt = Block::iterator(body.getTerminator());
 
   assert(insertPt->getParentOp() == symbolTableOp &&
          "expected insertPt to be in the associated module operation");
 
+  // The symbol cannot be the child of another op and must be the child of the
+  // symbolTableOp after this.
+  //
+  // TODO: consider if SymbolTable's constructor should behave the same.
+  if (!symbol->getParentOp())
     body.getOperations().insert(insertPt, symbol);
+  assert(symbol->getParentOp() == symbolTableOp &&
+         "symbol is already inserted in another op");
 
   // Add this symbol to the symbol table, uniquing the name if a conflict is
   // detected.
   StringRef name = getSymbolName(symbol);
   if (symbolTable.insert({name, symbol}).second)
     return;
   // If a conflict was detected, then the symbol will not have been added to
   // the symbol table. Try suffixes until we get to a unique name that works.