[PATCH] D21149: [CodeView] Add support for emitting S_UDT for typedefs

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:780
@@ +779,3 @@
+  }
+  if (ClosestSubprogram == CurrentSubprogram) {
+    std::string FullyQualifiedName;
----------------
aaboud wrote:
> majnemer wrote:
> > rnk wrote:
> > > majnemer wrote:
> > > > rnk wrote:
> > > > > Why do we need this condition? Don't we want to emit typdefs at global scope? I'd expect we can reference function local typedefs from outside the function, with something like return type deduction:
> > > > >   auto f() {
> > > > >     typedef int myty;
> > > > >     struct A {
> > > > >       myty a;
> > > > >     };
> > > > >     return A();
> > > > >   }
> > > > >   decltype(f()) gv;
> > > > > 
> > > > > Maybe that's contrived, but the reliance on type emission order here feels fragile.
> > > > > 
> > > > > I was envisioning that we'd have a vector of plain DIType*'s, and we'd iterate them at end of TU and call getCompleteTypeIndex on them, which would trigger emission of all non-forward decl record types.
> > > > MSVC emits typedefs nested within functions in the same symbol section as the one for the function, I was trying to make sure that this would happen.
> > > > Consider if `f` where `extern inline`: where should the `S_UDT` go?  I imagine MSVC will put it with the ProcSym for `f` which will be in a different section than the one for `gv`.
> > > > 
> > > > That being said, I think we can just never use the COMDAT section for the `S_UDT` records and always just stick them in the default section.
> > > > WTDYT?
> > > What if we staple UDTs into the relevant FunctionInfo? Currently FnDebugInfo is indexed by Function*, but it could easily be changed to DISubprogram since the Function points to that.
> > Hmm, I'm not sure how this helps.  Today, we add functions to `FnDebugInfo` at `beginFunction` and we process them at `endModule`.
> > 
> > When would we be doing the stapling? We will only see the types when we are processing entities like locals.  What if the local we are processing references a type that was defined in a prior `FnDebugInfo` entry?
> Not sure how this is possible.
> But assume it is, I am not sure how your code above solve the case!
> You are preventing emitting S_UDT in the wrong function, but you are ending up losing S_UDT because we are not processing types before we emitting symbols, which seems as a wrong design.
Here is how it is possible:
  extern inline auto f() {
    struct X { struct Y {}; };
    return X{};
  }
  void g() {
    decltype(f())::Y y;
  }

I never claimed that my code solved the case and I'm quite aware that we are potentially dropping S_UDTs where we shouldn't.  The question is: how do we fix it.


http://reviews.llvm.org/D21149