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); ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62167/new/ https://reviews.llvm.org/D62167 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
