[PATCH] Fixing a bug where debug info for a local variable gets emitted at file scope

[David Blaikie]

  Thanks for working/pinging this - I've reviewed it in more detail & it generally looks good, just a few open questions/cleanups.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3058
@@ +3057,3 @@
+  // where they are being referenced. Do not change the current source location
+  // to the place where they are declared, lest we get a bogus line table.
+  if (llvm::GlobalVariable *Var = dyn_cast<llvm::GlobalVariable>(VarOrInit)) {
----------------
This comment seems suspiciously like we're working around something in a not-great way, but I don't understand: why are we setting the location at all here, do you know or did you copy this from elsewhere?

================
Comment at: test/CodeGen/debug-info-line5.cpp:2
@@ +1,3 @@
+// RUN: %clangxx -g -gcolumn-info -O0 %s -c -o %t.o
+// RUN: llvm-dwarfdump %t.o | FileCheck %s
+// Line table entries should reference this cpp file, not the header
----------------
Clang tests should only test the generated IR without invoking the LLVM backend to emit object files, etc. If your change is purely in Clang, the should be possible to test it by only exercising Clang.

================
Comment at: test/CodeGen/debug-info-scope.c:28
@@ +27,3 @@
+
+int foo()
+{
----------------
Any reason you needed to add this 'foo' function as well as referencing 'd' at the end of the previous function? 

================
Comment at: test/CodeGenCXX/debug-info-namespace.cpp:39
@@ -38,1 +38,3 @@
 
+namespace monkey
+{
----------------
Thanks for putting all these test cases in reasonable existing files.

Though please try to be consistent with existing formatting in files (you could continue to use single letter class names "C", "D", etc rather than monkey/ape - simple, metasyntactic variables can be easy to follow rather than adding extra unnecessary semantics to the test cases) and usually with the LLVM style guide (so, '{' at the end of the line, rather than the beginning) 


http://llvm-reviews.chandlerc.com/D1281