[Lldb-commits] [PATCH] D17274: improve readability and performance of ClangExpressionParser::FindFunctionInModule

[Dawn Perchik via lldb-commits]

dawn accepted this revision.
dawn added a comment.
This revision is now accepted and ready to land.

lgtm other than nitpick on formatting.  And sorry for delayed review - been sick.


================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:559-560
@@ -558,5 +558,4 @@
 
-static bool FindFunctionInModule (ConstString &mangled_name,
-                                  llvm::Module *module,
-                                  const char *orig_name)
+static bool
+FindFunctionInModule(ConstString &mangled_name, llvm::Module *module, const char *orig_name)
 {
----------------
ldrumm wrote:
> jingham wrote:
> > Don't make this kind of change, please.  As long as the arguments fit in 120 characters we don't have a rule one way or the other about how to write argument lists like this.  But changing them just because they look better to you results in unnecessary churn.  Moreover, this is changing it away from the way all the other functions in this source file are written, so it ends up looking odd.
> This change is made by clang-fomat using the rules in the lldb .clang-format file.
> 
> I'm willing to revert this part of the commit, but seeing as this change is essentially a refactoring of the whole method, it feels natural to also format the prototype while I’m at it.
Please just keep the space before the params and the return type on a separate line.  Sadly, we can't use clang-format for function decls/defs in lldb because it doesn't support the lldb-style here, so the formatting of this must be done manually :(


http://reviews.llvm.org/D17274