[PATCH] D115325: [DWARF] Fix PR51087 Extraneous enum record in DWARF with type units

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1455-1461
+      if (DICompositeType *CTy = dyn_cast<DICompositeType>(Ty)) {
+        if (generateTypeUnits() && !CTy->isForwardDecl()) {
+          MDString *TypeId = CTy->getRawIdentifier();
+          if (TypeId && getOrCreateDwarfTypeUnit(*CU, TypeId->getString(), CTy))
+            continue; // Type unit added successfully.
+        }
+      }
----------------
dblaikie wrote:
> This could be pulled into a function or lambda and reused across the enum and retained types handling? (either just using the dyn_cast version for both, or keeping the dyn_cast/cast distinction between the two and everything inside/after that could be shared - I'd probably just keep the dyn_cast version and call that from both places. It could return a bool to indicate whether the thing has been handled or not.)
I opted for putting everything inside the new function, including the call to `getOrCreateTypeDIE` because it felt more clear; having a third wrapper around `addDwarfTypeUnitType` (e.g. calling and returning value of `getOrCreateDwarfTypeUnit` else returning false) looked a bit odd. Does this look okay to you?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115325/new/

https://reviews.llvm.org/D115325