[PATCH] D15139: [IR] Reformulate LLVM's EH funclet IR

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added inline comments.

================
Comment at: lib/Analysis/EHPersonalities.cpp:54
@@ +53,3 @@
+  // For any block B, the "colors" of B are the set of funclets F (possibly
+  // including a root "funclet" representing the main function), such that
+  // F will need to directly contain B or a copy of B (where the term "directly
----------------
JosephTremoulet wrote:
> rnk wrote:
> > I think the comment means that the list of colors may include the entry block, which isn't strictly speaking a funclet.
> Right, I'm pretty sure I wrote the comment, and that's what I meant -- that the colors *for any given block* may or may not include the root.
Oh, I see.  For some reason I read it as saying that the complete set of funclets for the function could "possibly" include the entry block, and it confused me because that will always be a color.  I see now that it is actually saying that the set of colors for any given block may (or may not) include the entry block.  That makes a lot more sense.

As a minor nit, the commas in this sentence shouldn't be there.  I'm going to blame my reading comprehension failure on that.

At the risk of sounding pedantic, it is a little ambiguous as to whether 'F' refers to the set of funclets that are colors for B or a single funclet in that set.  From context it has to be a single funclet, but grammatically I think it's the set.  I suggest this:

"For any block B the 'colors' of B are the set of funclets (possibly including a root 'funclet' representing the main function) such that each funclet F in that set will need to directly contain B or a copy of B...."


http://reviews.llvm.org/D15139