[PATCH] D15961: [WinEH] Verify unwind edges against EH pad tree

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added inline comments.

================
Comment at: docs/ExceptionHandling.rst:801
@@ +800,3 @@
+* Any of these pads is exited when control unwinds to the function's caller
+  (by a ``call``, ``cleanupret``, or ``catchswitch``).
+* Any of these pads is exited when an unwind edge (from an ``invoke``,
----------------
I found this line confusing.  The cleanupret and catchswitch parts of the parenthetical clause seem to be redundant, having already been covered by previous statements, so it took me a bit to figure out how they related here to the call statement.  I also think it would be better to leave the explicit verbiage explaining that a call may unwind to the parent function -- "implicitly via a call (which unwinds all the way to the current function's caller)".

================
Comment at: docs/LangRef.rst:5597
@@ +5596,3 @@
+which must be the label of another basic block beginning with a
+"pad" instruction, one of ``cleanuppad`` or ``catchswitch``.  This unwind edge
+must have a legal target with respect to the ``parent`` links, as described in
----------------
This would read better as "beginning with either a cleanuppad or catchswitch instruction".

================
Comment at: docs/LangRef.rst:5598
@@ +5597,3 @@
+"pad" instruction, one of ``cleanuppad`` or ``catchswitch``.  This unwind edge
+must have a legal target with respect to the ``parent`` links, as described in
+the `exception handling documentation\ <ExceptionHandling.html#wineh-constraints>`_.
----------------
Should "must have..." here be "must be..."?

================
Comment at: lib/IR/Verifier.cpp:2937
@@ +2936,3 @@
+  // pad relationship.
+  Instruction *ToPad = BB->getFirstNonPHI();
+  Value *ToPadParent = getParentPad(ToPad);
----------------
Won't this be the same as the 'I' argument?


http://reviews.llvm.org/D15961