[PATCH] D12108: [WinEH] Require token linkage in EH pad/ret signatures

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added a comment.

Updating just the accessors' return types didn't work like I'd hoped.  It would require the verifier to reject mismatches, but in order to do so with a proper error message both the verifier and the asm printer would have needed to get the bogus operand without calling `getFooPad()`, which meant either hardcoding the pad operand index in the verifier/printer (seems bad) or adding another method to the `FooReturnInst` classes returning the index or returning the pad operand as a `Value *`, at which point the API surface on the `FooRetInstr`s looked pretty convoluted.  I also explored using a `dynamic_cast` in the parsers, but of course that just leads to the same issues with the verifier/printer.  So I think that if we don't want the `FooReturnInst` ctors/accessors/mutators to all take `Value *` (which certainly seems in-line with keeping the IR approachable), the only thing to do that doesn't involve horrible hacks is to teach the parsers that some forward references need to be `FooPad`s.  I will look into doing that, but let me know if that seems off-base, and feel free to suggest something better.


================
Comment at: docs/ExceptionHandling.rst:676
@@ -676,3 +675,3 @@
   lpad.cleanup:                                     ; preds = %invoke.cont
-    cleanuppad [label %lpad.catch]
+    %cleanup = cleanuppad
     call void @"\01??_DCleanup@@QEAA at XZ"(%struct.Cleanup* nonnull %obj) nounwind
----------------
majnemer wrote:
> Shouldn't the `cleanuppad` be followed by `[]`
Yep, thanks.

================
Comment at: docs/ExceptionHandling.rst:681
@@ -681,3 +680,3 @@
   lpad.catch:                                       ; preds = %entry, %lpad.cleanup
-    catchpad void [%rtti.TypeDescriptor2* @"\01??_R0H at 8", i32 0, i32* %e]
+    %catch = catchpad void [%rtti.TypeDescriptor2* @"\01??_R0H at 8", i32 0, i32* %e]
             to label %catch unwind label %lpad.terminate
----------------
majnemer wrote:
> Should the `void` be removed?
Yes, updated.


http://reviews.llvm.org/D12108