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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
