[PATCH] D17580: Extract the method to begin and end a fragment in AsmPrinterHandler in their own method. NFC

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfCFIException.cpp:149
@@ +148,3 @@
+  auto *P = dyn_cast<Function>(F->getPersonalityFn()->stripPointerCasts());
+  assert(P && "Expected personality function");
+
----------------
if you do not assert F->hasPersonalityFn(),  can F->getPersonalityFn() possibly return NULL -- then you will get segfault which is less desirable than asserting.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfException.h:67
@@ +66,3 @@
+
+  void beginFragment(const MachineBasicBlock *MBB,
+                     ExceptionSymbolProvider ESP) override;
----------------
./lib/CodeGen/AsmPrinter/ARMException.cpp:39:ARMException::ARMException

ARMException::ARMException(AsmPrinter *A) : DwarfCFIExceptionBase(A) { ..)

ARMException class also derives from DwarfExceptionBase. If beginFragment()/endFragment() is not pushed into the base class, this class will have inherited the empty definition which is wrong. 

Correction -- only endFragment part is wrong -- endFragment is called in the base class implementation ...


http://reviews.llvm.org/D17580