[Lldb-commits] [PATCH] D79447: [Debug][CodeView] Emit fully qualified names for globals

[Reid Kleckner via Phabricator via lldb-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, modulo ifdef adjustment



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3125-3126
   } else {
     // FIXME: Currently this only emits the global variables in the IR metadata.
     // This should also emit enums and static data members.
     const DIExpression *DIE = CVGV.GVInfo.get<const DIExpression *>();
----------------
aganea wrote:
> akhuang wrote:
> > rnk wrote:
> > > @akhuang, can we remove this FIXME? I think we addressed this simply by changing clang to emit this metadata.
> > Yep- thanks for catching that!
> Can I leave the comment for 'enum'? There are still a few things missing in the debug stream, in regards to enums (see other comment).
I'd leave the FIXME out, as you have it now. I don't think we're getting value from it here. At this point, it's in the frontend's hands to decide if the enum needs emitting, so the code fix would be far from this comment.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2989
+    const std::vector<std::pair<std::string, const DIType *>> &UDTs) {
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+  size_t OriginalSize = UDTs.size();
----------------
This probably needs to be `#ifndef NDEBUG`, or a build with asserts but without ABI breaking checks (blech) will break. Besides, it's not an ABI breaking check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79447