[PATCH] D62167: CodeView - add static data members to global variable debug info.

[Amy Huang via Phabricator via llvm-commits]

akhuang marked an inline comment as done.
akhuang added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385
+    // Use the global scope for static members.
+    DContext = getContextDescriptor(
+       cast<Decl>(CGM.getContext().getTranslationUnitDecl()), TheCU);
----------------
akhuang wrote:
> dblaikie wrote:
> > akhuang wrote:
> > > @dblaikie I'm using the global scope here because if the class type is used as the scope it runs into the [[ https://github.com/llvm/llvm-project/blob/a2ee80b084e5c0b20364ed4379384706f5e059b1/llvm/lib/IR/DIBuilder.cpp#L630 | assert message ]] `Context of a global variable should not be a type with identifier`. Is there a reason for the assert? 
> > I think it's generally how LLVM handles definitions for the most part - putting them at the global scope.
> > 
> > Though I'm confused by this patch - it has a source change in clang, but a test in LLVM. Generally there should be testing to cover where the source change is, I think?
> yep, there should be a test for this - added now. 
So for the global scope, it seems like [[ https://github.com/llvm-mirror/clang/blob/900624ef605b60c613342dac071228539a402ce9/lib/CodeGen/CGDebugInfo.cpp#L3274 | elsewhere ]], when creating the debug info for a static data member global variable, the scope is set to be the global scope. But it would be helpful for codeview debug info if the scope remained as the class type, partially so we can use the class type information in the variable name that we emit.

Does it seem reasonable to remove the assert for global variable scope so that the scope can be the class here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62167