[mlir] Clean the state after the op is erased.

[jpienaar]

Could you expand the description? This seems fine, but not sure why it is needed: it would seem to be where we keep an Operation around/operating on it post erasing it, that seems like a bug calling side. Setting nullptr probably would result in this failing more explicitly, but don't know if we can/should attempt to provide much guarantees at this low level.

[mehdi_amini]

I'd like to understand a bit more the motivation here.
It is a change in behavior I believe for this kind of code:

SomeOp op = ...
...
op.erase();
...
if (op) // null check here does not trigger now but will with this patch.

However I'm not sure it is desirable because op->erase() won't behave the same way.
(Actually I thought all of the method forwarding to the state were already removed)

[Chia-hungDuan]

Yes, that's about the case mentioned by @mehdi_amini.

I used OwningOpRef to hold the ModuleOp in a class like

class Foo {
// ...
OwningOpRef ref;
// ...
};

and somehow I wanted to clean this ModuleOp before the destruction of Foo. I did it as ref.release()->erase() so it's safe of later null check in OwningOpRef. Then I was thinking that if someone just called ModuleOp::erase() without using ref.release()->erase() then that would be a problem.

After thinking it a little bit more, Op<...> is just a wrapper (nothing relates to the life cycle management of the underlying object) and should be viewed as a raw pointer. As a result, the above case should be handled properly by the user.

I think this can be closed.

mlir/include/mlir/IR/OpDefinition.h

public:
 
   /// The source location the operation was defined or derived from.
   Location getLoc() { return state->getLoc(); }
 
   /// Return true if there are no users of any results of this operation.
   bool use_empty() { return state->use_empty(); }
 
   /// Remove this operation from its parent block and delete it.
-  void erase() { state->erase(); }
+  void erase() {
+    state->erase();
+    state = nullptr;
+  }
 
   /// Emit an error with the op name prefixed, like "'dim' op " which is
   /// convenient for verifiers.
   InFlightDiagnostic emitOpError(const Twine &message = {});
 
   /// Emit an error about fatal conditions with this operation, reporting up to
   /// any diagnostic handlers that may be listening.
   InFlightDiagnostic emitError(const Twine &message = {});