[PATCH] D11041: New EH representation for MSVC compatibility

[David Majnemer]

majnemer marked 5 inline comments as done.
majnemer added a comment.

In http://reviews.llvm.org/D11041#201942, @JosephTremoulet wrote:

> Also, is there any new UB that should be spelled out in the documentation?  E.g. can you return (from the function) from within a catchblock (without first executing catchret), and likewise for cleanups?  Unwind from a catchblock to somewhere other than its catchendblock?  Catchret from a cleanupblock or cleanupret from a catchblock?


Yes, those should all be UB.  I'll add them to the LangRef.


================
Comment at: docs/LangRef.rst:5123-5125
@@ +5122,5 @@
+
+The ``catchblock`` must be provided a ``normal`` label to transfer control
+to if the ``catchblock`` matches the exception and an ``exception``
+label to transfer control to if it doesn't.
+
----------------
JosephTremoulet wrote:
> What should the exception label look like for a catch that is unconditionally entered (like catch(...) in C++)?  Or do front-ends need to translate such things to cleanups?
The catchblock should unwind to a catchendblock in that case.

================
Comment at: docs/LangRef.rst:5140
@@ +5139,3 @@
+
+The ``catchblock`` instruction has several restrictions:
+
----------------
JosephTremoulet wrote:
> Is there also a restriction on the target of the exception label?  E.g. that it must be a catchblock or catchendblock?
Yes.

================
Comment at: docs/LangRef.rst:5184
@@ +5183,3 @@
+in-flight exception.
+
+If a ``nextaction`` label is not present, the instruction unwinds out of
----------------
JosephTremoulet wrote:
> If we have
> ```
> try {
>    code;
>    try {
>      code;
>    } catch (T e) {  // <--- catch 1
>      MayThrow();
>    }
>    code;
> } catch (T2 e2) {  // <---- catch 2
>   code;
> }
> ```
> 
> is the invoke for MayThrow supposed to target the catchbolck for catch2, or the catchendblock for catch1?  If it's the catchendblock, having some verbiage here explaining that would be good.
MayThrow should unwind to the same catchendblock that "catch 1" will unwind to.  That catchendblock would unwind to the outer catchblock.

================
Comment at: docs/LangRef.rst:5265
@@ +5264,3 @@
+
+.. _i_cleanupret:
+
----------------
JosephTremoulet wrote:
> nit: You introduce cleanupret before you introduce cleanupblock; that reads strangely.
cleanupblock isn't a terminator and the terminators are all documented together :/

================
Comment at: lib/IR/Instruction.cpp:468-471
@@ -457,2 +467,6 @@
     return !CI->doesNotThrow();
+  if (const auto *CRI = dyn_cast<CleanupReturnInst>(this))
+    return CRI->unwindsToCaller();
+  if (const auto *CEBI = dyn_cast<CatchEndBlockInst>(this))
+    return CEBI->unwindsToCaller();
   return isa<ResumeInst>(this);
----------------
JosephTremoulet wrote:
> Should there be a clause for TerminateBlock here?
I think TerminateBlock is more like a function-call which never returns and thus wouldn't throw.


http://reviews.llvm.org/D11041