Hidden panic in `OpType` API

[zrho]

There is a blanket implementation From<T> for OpType for all T: MakeRegisteredOp. This implementation hides panics that can occur when the T is not valid. Because these are panics on invalid inputs to the API and not internal errors, this is a bug in the API design.

Ideally the API should acknowledge that the conversion can fail by implementing TryFrom instead. If TryFrom is too inconvenient, the panic should include a helpful message and not simply unwrap an Option. Moreover the fact that this API can panic (and the cases in which it does) should be documented.

When does it panic?

The panic can happen in <OpType as From<T>>::from when unwrapping the Option returned by <T as MakeRegisteredOp>::to_extension_op. The default implementation of MakeRegisteredOp::to_extension_op uses the blanket implementation From<T> for RegisteredOp for all T: MakeRegisteredOp and then calls <RegisteredOp as MakeRegisteredOp>::to_extension_op. This method has three ways in which a None can be returned:

• The Weak<Extension> is upgraded. Since it is obtained from <T as MakeRegisteredOp>::extension_ref it could be considered an internal error, since valid implementations of MakeRegisteredOp should never return extension references that can't be upgraded. However, this requirement is not documented in MakeRegisteredOp::extension_ref. Moreover, the upgrade failing should panic with a message that indicates that this is happened.
• Extension::get_op might fail given the operation id reported by <T as MakeExtensionOp>::op_id. Analogous to the previous case, this indicates an erroneous implementation and therefore should at least be indicated by a panic with an explicit message.
• Finally, ExtensionOp::new can return a SignatureError in case that the type arguments passed to the operation are invalid. The signature error is dropped by converting the Result to an Option. This is a case of invalid user input: I've seen this fail in tket2 when constructing an invalid WasmOp. This error should be propagated.

[zrho]

I think implementations of MakeRegisteredOp should be required to return a reference to an Extension that is guaranteed to exist. Every implementation of MakeRegisteredOp::extension_ref in hugr and tket2 calls Arc::downgrade. I'd therefore propose that MakeRegisteredOp::extension_ref is changed to return an Arc<Extension> instead of a Weak<Extension>, which would indicate this requirement in the types and removes the need for an unwrap.

A potential reason to not change this to Arc<Extension> would be if MakeRegisteredOp::extension_ref is required to work while constructing the Extension using Arc::new_cyclic. This is not the case currently in hugr and tket2 and I do not know if it is intended to be used in that way. If it is, we should document this.