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.

On Wed, Mar 14, 2018 at 11:34 AM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> Except for the to -> To to keep consistent with all the other lldb
> function naming this looks fine.
>
> 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.
>
>
>
> ================
> Comment at: include/lldb/Interpreter/OptionArgParser.h:18-19
> +struct OptionArgParser {
> +  static lldb::addr_t toAddress(const ExecutionContext *exe_ctx,
> +                                llvm::StringRef s, lldb::addr_t
> fail_value,
> +                                Status *error);
> ----------------
> Should be ToAddress.
>
>
> https://reviews.llvm.org/D44306
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to