[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

This revision was automatically updated to reflect the committed changes. Closed by commit rL329677: Move Args::StringTo*** functions to a new OptionArgParser class (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44306

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. Thank you. PS: I will need someone to update the XCode project after this. https://reviews.llvm.org/D44306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Yes, with your updated naming, I am fine with this. https://reviews.llvm.org/D44306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. Jim, judging by the comment in https://reviews.llvm.org/D44306#1037587 you were fine with this patch except the naming part. Now that that's fixed, I hope this should be fine. Unless I hear from you, I am going to commit this so I can continue with re-layering the `Args`

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. So, any objections to this patch? https://reviews.llvm.org/D44306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath updated this revision to Diff 138540. labath added a comment. s/toAddress/ToAddress/ https://reviews.llvm.org/D44306 Files: include/lldb/Interpreter/Args.h include/lldb/Interpreter/OptionArgParser.h include/lldb/Interpreter/Options.h source/API/SBDebugger.cpp source/Commands/Co

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. I agree with Jim. I deliberately put here only the types that are used for command parsing, since I presume these are the things that the Command/Interpreter modules will depend on (it's not fully clear to me where to draw the line between these two, so it may end up nee

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

jingham added a comment. As long as these functions are used for parsing options args in individual CommandObject implementations, and don't get dragged lower in the stack, I don't think that's a problem. By the time you're getting to individual commands you are on top of pretty much everythin

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

zturner added a subscriber: labath. zturner added a comment. 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. https://reviews.llvm

Re: [Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

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.

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

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 referenc

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath updated this revision to Diff 138418. labath added a comment. Herald added a subscriber: mgorny. This is a version that moves the StringTo*** functions to a new "OptionArgParser" class. I'm not terribly proud of the name, but I couldn't think of anything better -- we already have a OptionPa

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath planned changes to this revision. labath added a comment. Yes, that thought occurred to me almost immediately after I submitted this. I'll try to implement something like that instead. Since this will be just a bunch of static functions with no state, I think we don't even have to put th

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

jingham added a comment. I liked the fact that all these Argument input type string converters were all in one place, though arguably Args was not the right place. Among other things it made it easy to see that if you were adding a new command that took in an Address, AddressToString was the c

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath created this revision. labath added reviewers: jingham, zturner. StringToAddress could end up evaluating an expression, which is a somewhat unexpected behavior of a seemingly simple method. I rename it to EvaluateAddressExpression and move it to the Target class, next to the standard Evalua