[PATCH] Fixed debug info generation for function static variables, typedef, and records

[David Blaikie]

================
Comment at: include/llvm/ADT/iterator_range.h:42
@@ -41,1 +41,3 @@
+
+  bool empty() const { return begin() == end(); }
 };
----------------
I wouldn't add this here - the range concept is just "begin" and "end" - so there may be other ranges that don't have "empty". So we don't want to make our range-based algorithms overly restrictive by requiring that their ranges have 'empty'.

Instead this should be a free function:

  template<typename R>
  bool empty(const R& r) { return begin(r) == end(r); }

================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:357
@@ +356,3 @@
+      for (const auto &DI : LocalDeclNodeRangeForScope) {
+        if (auto IE = dyn_cast<DIImportedEntity>(DI.second))
+          Children.push_back(constructImportedEntityDIE(IE));
----------------
Please include the '*' for auto bindings to pointer type. ie:

  if (auto *IE = dyn_cast<DIImportedEntity>(DI.second))

etc

================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:81
@@ -80,3 +80,3 @@
 
-  /// getOrCreateGlobalVariableDIE - get or create global variable DIE.
-  DIE *getOrCreateGlobalVariableDIE(const DIGlobalVariable *GV);
+  /// \brief get or create global variable DIE.
+  /// \param GV Global Variable Node
----------------
I think we have auto-brief on now, so there's no need to put \brief there.

Also, capitalize 'g' in get.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:454
@@ +453,3 @@
+    for (auto *GV : CUNode->getGlobalVariables()) {
+      if (CU.getOrCreateContextDIE(GV->getScope()))
+        CU.getOrCreateGlobalVariableDIE(GV);
----------------
Do we need to actually try to build the context here or is there something cheaper/more specific we could do? Check the node type of the scope To see if it's a lexical or function scope, perhaps?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:471
@@ +470,3 @@
+      auto *Context = resolve(Ty->getScope());
+      if (CU.getOrCreateContextDIE(Context))
+        CU.getOrCreateTypeDIE(RT);
----------------
same here

================
Comment at: test/DebugInfo/LB-class.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -filetype=obj -O0 < %s  | llvm-dwarfdump -debug-dump=info - | FileCheck %s
+
----------------
What's the "LB" suffix in these tests?

It seems like these could all be in a single test, probably? The tests should probably be simpler, too. Like:

  void func() {
    {
    struct foo {
    };
    foo f;
    typedef int bar;
    bar b;
    }
  }

etc?

http://reviews.llvm.org/D9758

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/