[Lldb-commits] [PATCH] D43837: Move option parsing out of the Args class

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

It would be nice if we could eventually get rid of the need to pass in a platform and execution context here, but that's work for another day.



================
Comment at: include/lldb/Interpreter/Options.h:123-126
+  llvm::Expected<Args> Parse(const Args &args,
+                             ExecutionContext *execution_context,
+                             lldb::PlatformSP platform_sp,
+                             bool require_validation);
----------------
labath wrote:
> zturner wrote:
> > It appears that all of these could be static functions.  Can we do that?
> They can't be. All of them access the `this` object. If you look at the original functions, they were taking an `Options&` as an argument and `Args` as `this`. These have that inverted.
I originally searched for `m_` and didn't find anything so assumed they could be static.  But it looks like they are calling member functions, which is why I didn't see it.  It's too bad it can't even be `const`, given that it returns a copy of the args.  Seems like an awkward interface, maybe future cleanup can try to tackle that though.  Anyway, ignore my comment.


https://reviews.llvm.org/D43837