[llvm] [SandboxIR] Remove tight-coupling with LLVM's SwitchInst::CaseHandle (PR #167093)

[via llvm-commits]

================
@@ -1884,45 +1884,96 @@ class SwitchInst : public SingleLLVMInstructionImpl<llvm::SwitchInst> {
     return cast<llvm::SwitchInst>(Val)->getNumCases();
   }
 
-  using CaseHandle =
-      llvm::SwitchInst::CaseHandleImpl<SwitchInst, ConstantInt, BasicBlock>;
-  using ConstCaseHandle =
-      llvm::SwitchInst::CaseHandleImpl<const SwitchInst, const ConstantInt,
-                                       const BasicBlock>;
-  using CaseIt = llvm::SwitchInst::CaseIteratorImpl<CaseHandle>;
-  using ConstCaseIt = llvm::SwitchInst::CaseIteratorImpl<ConstCaseHandle>;
+  class CaseIt;
+  class CaseHandle {
----------------
vporpo wrote:

OK I tried wrapping an `llvm::SwitchInst::CaseHandle` within a SandboxIR CaseHandle, but I don't think it can be done. The reason is that `llvm::SwitchInst::CaseHandle` is not publicly default constructible because it would contain invalid state (as it's job is to point to a particular case inside the switch). But the problem is that SandboxIR CaseIt needs to own a default-constructible SandboxIR CaseHandle object because of SandboxIR `CaseHandle & CaseIt::operator*()` needs to return a reference to CaseHandle, not a copy (i.e., in `CaseIt::operator*()` we can't just create a local CaseHandle object and return it).

But design-wise having SandboxIR CaseHandle wrap an llvm::SwitchCase::iterator isn't actually too weird. The CaseHandle object needs to be able to point to a particular case in the switch, and to also provide the corresponding interface functions like getCaseIndex(), getSuccessor() etc. The "pointing to particular case" part is taken care of by the `llvm::SwitchCase::iterator`. Also modifying the CaseHandle's members from within CaseIt is pretty much what LLVM IR is doing too (e.g., operator++ increments CaseHandle::Index) because it is the LLVM CaseHandle that actually owns the state.

Wdyt?

https://github.com/llvm/llvm-project/pull/167093