[PATCH] Fully handle globals and functions in CGDebugInfo::getDeclarationOrDefinition()

[David Blaikie]

Looks pretty reasonable. Some optional and/or follow-up patch comments.

Have you considered if/how some of this could be unified with the type decl-or-def handling?

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2310
@@ -2309,1 +2309,3 @@
 
+void CGDebugInfo::collectFunctionDeclProps(GlobalDecl GD,
+                                           llvm::DIFile Unit,
----------------
If you want to submit these refactorings separately, feel free (& do that sort of thing without precommit review - just pulling out a bit of functionality and giving it a name is usually goodness even before the second user exists (we tend to have long functions that are hard to read - pulling out parts of them and giving them names usually makes them a bit easier to follow) - and it makes the code reviews more palatable (I can easily glance at a single function refactor like this, and mostly just assume you did the obvious thing of copy/paste the code and give it a name))

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2310
@@ -2309,1 +2309,3 @@
 
+void CGDebugInfo::collectFunctionDeclProps(GlobalDecl GD,
+                                           llvm::DIFile Unit,
----------------
dblaikie wrote:
> If you want to submit these refactorings separately, feel free (& do that sort of thing without precommit review - just pulling out a bit of functionality and giving it a name is usually goodness even before the second user exists (we tend to have long functions that are hard to read - pulling out parts of them and giving them names usually makes them a bit easier to follow) - and it makes the code reviews more palatable (I can easily glance at a single function refactor like this, and mostly just assume you did the obvious thing of copy/paste the code and give it a name))
Though the name 'collect' might be different from the existing similar functions we have for subprograms - could you check if we can have a more consistent naming one way or the other? (I think I named the subprogram ones 'apply' rather than 'collect')

Oh... I was confused, thought this was an LLVM patch (since it was sent to llvm-commits) so I was talking about LLVM codegen/asmprinter stuff... 

Nevermind.

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2406
@@ +2405,3 @@
+  if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
+    llvm::DIArray TParamsArray;
+    unsigned Flags = 0;
----------------
Might be worth splitting the body of the if into its own function (& for symmetry, though it's shorter, the body of the else separately as well) so this function just immediately defers to variable or function handlers.

================
Comment at: lib/CodeGen/CGDebugInfo.h:418
@@ -413,1 +417,3 @@
 
+  /// \brief Collect various properties of a FucntionDecl.
+  /// \param GD  A GlobalDecl whose getDecl() must return a FunctionDecl.
----------------
Typo of 'Fucntion'

http://reviews.llvm.org/D6173