[PATCH] D63012: Use fully qualified name when printing S_CONSTANT records

[Amy Huang via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3080-3081
+    // Enums defined in classes should also get scope from the class.
+    if (const auto *EnumTy = DIGV->getType())
+      if (const auto *EnumScope = EnumTy->getScope())
+        Scope = EnumScope;
----------------
rnk wrote:
> akhuang wrote:
> > rnk wrote:
> > > This seems like it would do the wrong thing for a regular constant that isn't an enum, but which has an enum type. This kind of thing:
> > >   namespace Bar {
> > >   enum Foo { FooA, FooB };
> > >   }
> > >   const Bar::Foo foo_gv = Bar::FooA;
> > > ... might come out as Bar::foo_gv when it should be just foo_gv.
> > > 
> > > I'm not actually sure what to do in this case, because we made enumerators look a lot like const ints, that was the whole idea. I mean, we could change clang to make them look like static const data members, I guess, but that seems like a step too far.
> > It happens to do the right thing here because the type for global constants is a const tag.. but this seems kind of unreliable.
> > 
> > Other than making them look like static const data members or adding some enum type to the DIGlobalVariable, I can't really think of a good way to do this other than making the scope not global?
> Making the scope not global seems like the most logical thing to me. It sounds like there's no particularly good reason why we use the global scope for static data members, other than that's the DWARF we want to emit, so we can go ahead and revise that decision for enums.
Actually, enums in classes currently have global scope, so maybe that won't work


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63012