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

[David Blaikie]

  I haven't looked at the details yet, sorry - just a cursory glance (feedback may not necessarily require any code changes, but things I noticed/questions I had on a brief look through)


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3051
@@ -3048,1 +3050,3 @@
   unsigned LineNo = getLineNumber(D->getLocation());
+  llvm::GlobalVariable *Var = dyn_cast_or_null<llvm::GlobalVariable>(VarOrInit);
+  bool IsLocalToUnit = Var ? Var->hasInternalLinkage() : true;
----------------
Can "VarOrInit" ever be null? If so, can you explain when/why? If not, use "dyn_cast" instead of dyn_cast_or_null.

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3052
@@ -3049,1 +3051,3 @@
+  llvm::GlobalVariable *Var = dyn_cast_or_null<llvm::GlobalVariable>(VarOrInit);
+  bool IsLocalToUnit = Var ? Var->hasInternalLinkage() : true;
 
----------------
I'd probably write this as:
bool isLocalToUnit = true;
if (Var) {
  setLocation(D->getLocation());
  isLocalToUnit = Var->hasInternalLinkage();
}

& if possible, even push the LinkageName stuff into the same conditional:

if (GlobalVariable *Var = dyn_cast...) {
  setLocation
  isLocalToUnit = ...
  if (D->getDeclContext() && !isa<functionDecl> ... )
    LinkageName = Var->getName();
}

perhaps. Not entirely sure of any of these transformations - just ideas, I haven't looked at all the code in the full context.


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