<div dir="ltr">I think having all parsing functions in a single place will just move the layering problem elsewhere, since a bunch of conversion functions for objects from various libraries will be mushed together into one place.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Mar 14, 2018 at 11:34 AM Jim Ingham via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">jingham added a comment.<br>
<br>
Except for the to -> To to keep consistent with all the other lldb function naming this looks fine.<br>
<br>
Now that they are all together it's easy to see we haven't been consistent in these functions.  We really should make ToFormat return the format & take an error reference, and have ToBoolean take an error so callers don't have to cons it up.  But that's orthogonal to this patch.<br>
<br>
<br>
<br>
================<br>
Comment at: include/lldb/Interpreter/OptionArgParser.h:18-19<br>
+struct OptionArgParser {<br>
+  static lldb::addr_t toAddress(const ExecutionContext *exe_ctx,<br>
+                                llvm::StringRef s, lldb::addr_t fail_value,<br>
+                                Status *error);<br>
----------------<br>
Should be ToAddress.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D44306" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44306</a><br>
<br>
<br>
<br>
</blockquote></div>