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

[Amaury SECHET via llvm-commits]

deadalnix 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");
+
----------------
davidxl wrote:
> if you do not assert F->hasPersonalityFn(),  can F->getPersonalityFn() possibly return NULL -- then you will get segfault which is less desirable than asserting.
shouldEmitPersonality already gates this, see line 115

================
Comment at: lib/CodeGen/AsmPrinter/DwarfException.h:67
@@ +66,3 @@
+
+  void beginFragment(const MachineBasicBlock *MBB,
+                     ExceptionSymbolProvider ESP) override;
----------------
davidxl wrote:
> ./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 ...
Good catch. Let em fix this.


http://reviews.llvm.org/D17580