[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

> > This LGTM!
> > I don't think I can see far enough ahead on what you are planning here, but I was just wondering if the ultimate goal is to have the `option_arg.getAsT` return an `Expected<T>`. In this case, wouldn't all these arguments (short option, long option, additional context) have to be part of the interface of `getAsT`? I suspect this is not your goal, but I can't see a way around that
> 
> The ultimate goal is to have option parsing be much more streamlined and automated. The majority of option parsing is just taking some primitive value and trying to grab its value from a string. There are places where we need additional context (e.g. in this PR, there's a place where we want to select a thread ID so there's more validation needed). I want to be able to write those pretty naturally too.
> 
> The first step in my plan is to centralize how we create error messages. The majority of parsing errors will be "I couldn't turn this into a boolean/integer/address/other", so I'm going to create something that is better than what we have today. The step after that will be to revamp the parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the way we do today, there will be something like `llvm::Expected<bool> value_or_error = ParseAsBool(option);` where `option` is the actual OptionDefinition. I'm not tied to these abstractions, but I am committed to the path of making this easier to write and maintain.


Ohhh I think I see it now!

> where `option` is the actual OptionDefinition

And the `option` itself would know how to report / construct the error strings?


https://github.com/llvm/llvm-project/pull/82273