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

[Luke Drummond via lldb-commits]

ldrumm added inline comments.

================
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)
 {
----------------
dawn wrote:
> 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 :(
Thanks for your comments, Dawn. I appreciate the time it takes - especially after being ill :(

@jingham @dawn
To step around the style questions completely, I've updated the changelist to ignore the prototype entirely. I hope that suits you both.

Thanks again


Repository:
  rL LLVM

http://reviews.llvm.org/D17274