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

[Yunzhong Gao]

  Hi David,
  Thanks for reviewing my patch. I try to address your concerns below.

  > lib/CodeGen/CGDebugInfo.cpp#3058
  > 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?

  I do not know either. In the original codes, EmitGlobalVariable() for
  non-deferred global variables sets source locations; the version for deferred
  global variables does not set source location. So when I combine these two
  versions in this patch, I added a condition check to preserve the original
  code semantics. I tried to disable this particular line of codes, and the
  test results in LLVM/Clang regression suites were still clean...

  But on the other hand, I think this is not really relevant to this bug, and
  if we decide to remove this line, it should be done in a separate patch. Is
  it okay to add a FIXME comment there for now?

  > test/CodeGen/debug-info-line5.cpp#2
  > 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.

  Thank you for pointing it out. Fixed.

  > test/CodeGen/debug-info-scope.c#28
  > Any reason you needed to add this 'foo' function as well as referencing
  > 'd' at the end of the previous function?

  Not really. I removed this local function.

  > test/CodeGenCXX/debug-info-namespace.cpp#39
  > 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)

  Fixed (hopefully).

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1281?vs=3583&id=3907#toc

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/debug-info-scope.c
  test/CodeGen/debug-info-line5.h
  test/CodeGen/debug-info-line5.cpp
  test/CodeGenCXX/debug-info-namespace.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1281.5.patch
Type: text/x-patch
Size: 12015 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/043d32ba/attachment.bin>