rnk added inline comments.

================
Comment at: include/clang/Basic/LangOptions.def:111
@@ -110,2 +110,3 @@
 LANGOPT(SjLjExceptions    , 1, 0, "setjmp-longjump exception handling")
+LANGOPT(NewMSEH           , 1, 0, "new IR representation for MS exceptions")
 LANGOPT(TraditionalCPP    , 1, 0, "traditional CPP emulation")
----------------
Seems like it should be a CodeGenOption.

================
Comment at: lib/CodeGen/CGCleanup.cpp:907
@@ +906,3 @@
+    llvm::BasicBlock *NextAction = getEHDispatchBlock(EHParent);
+    if (CGM.getLangOpts().NewMSEH && getTarget().getCXXABI().isMicrosoft()) {
+      if (NextAction)
----------------
Should we do anything to synchronize these getCXXABI checks with the 
EHPersonality? At the very least, I think it's more intuitive to the reader to 
make the decisions about the IR we use to be based on the personality.

Probably the right thing to do here is to declare EHPersonality in CGCleanup.h, 
and then acknowledge that CGCleanup.h is actually CGException.h. Sounds like a 
separable patch.

================
Comment at: lib/CodeGen/CGException.cpp:897
@@ +896,3 @@
+                                                    EHCatchScope &catchScope) {
+  llvm::BasicBlock *dispatchBlock = catchScope.getCachedEHDispatchBlock();
+  assert(dispatchBlock);
----------------
Should we rename the locals here according to the prevailing LLVM style?

================
Comment at: lib/CodeGen/CGException.cpp:911
@@ +910,3 @@
+    llvm::Value *typeValue = handler.Type;
+    assert((typeValue != nullptr || handler.isCatchAll()));
+    if (!typeValue)
----------------
Drop the extra parens?

================
Comment at: lib/CodeGen/CGException.cpp:944
@@ -824,3 +943,3 @@
 /// It is an invariant that the dispatch block already exists.
-static void emitCatchDispatchBlock(CodeGenFunction &CGF,
-                                   EHCatchScope &catchScope) {
+static llvm::BasicBlock *emitCatchDispatchBlock(CodeGenFunction &CGF,
+                                                EHCatchScope &catchScope) {
----------------
Please add a comment documenting what the return value is. I'm thinking:
  If the catchblock instructions are used for EH dispatch, then the basic block 
holding the final catchendblock instruction is returned.


http://reviews.llvm.org/D11405




_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to