Excellent. Jim
> On Jul 3, 2014, at 12:41 PM, Zachary Turner <[email protected]> wrote: > > Yea, in this CL I wasn't yet sure what would be the best argument to have the > method take, so I just had it take nothing. You caught in my follow-up CL > that I used CommandInterpreter though, so I'll make sure to change it to > ExecutionContext before pushing it. > > > On Thu, Jul 3, 2014 at 10:43 AM, <[email protected]> wrote: > > > On Jul 3, 2014, at 10:41 AM, Greg Clayton <[email protected]> wrote: > > > > Looks fine. One question: > > > > In the definition of the OptionValidator: > > > > struct OptionValidator > > { > > virtual ~OptionValidator() { } > > virtual bool IsValid() const = 0; > > virtual const char * ValidConditionString() const = 0; > > }; > > > > Should the IsValid() and ValidConditionString() condition string take a > > "CommandInterpreter &command_interpreter" as an argument? This would allow > > us to get to the current execution contents (selected > > target/process/thread/frame), mainly so we can get the lldb_private::Target > > so we can get the platform from it so we can correctly make the > > determination if the option should be valid or not for the current context. > > I think rather that they should take an ExecutionContext. You can't depend > on the command interpreter, since the selected target in the command > interpreter might be TargetA, but we are executing the breakpoint action > commands for a breakpoint hit on TargetB. But I assume Zachary just hadn't > gotten to this yet. > > Jim > > > > > > Greg > > > >> On Jul 3, 2014, at 12:33 AM, Zachary Turner <[email protected]> wrote: > >> > >> This patch adds the notion of an OptionValidator to the OptionDefinition. > >> The purpose of the OptionValidator is to determine, based on some > >> arbitrary set of conditions, whether or not a command option is valid for > >> a given debugger state. An example of this might be to selectively > >> disable or enable certain command options that don't apply to a particular > >> platform. > >> > >> This patch contains no functional change, and does not actually make use > >> of an OptionValidator for any purpose yet. It involves alot of code > >> churn, however, so is submitted independently so as not to muddy up the > >> subsequent change which actually begins making use of the validator. > >> > >> http://reviews.llvm.org/D4369 > >> > >> Files: > >> include/lldb/lldb-private-types.h > >> source/Commands/CommandObjectArgs.cpp > >> source/Commands/CommandObjectBreakpoint.cpp > >> source/Commands/CommandObjectBreakpointCommand.cpp > >> source/Commands/CommandObjectCommands.cpp > >> source/Commands/CommandObjectDisassemble.cpp > >> source/Commands/CommandObjectExpression.cpp > >> source/Commands/CommandObjectFrame.cpp > >> source/Commands/CommandObjectHelp.cpp > >> source/Commands/CommandObjectLog.cpp > >> source/Commands/CommandObjectMemory.cpp > >> source/Commands/CommandObjectPlatform.cpp > >> source/Commands/CommandObjectProcess.cpp > >> source/Commands/CommandObjectRegister.cpp > >> source/Commands/CommandObjectSettings.cpp > >> source/Commands/CommandObjectSource.cpp > >> source/Commands/CommandObjectTarget.cpp > >> source/Commands/CommandObjectThread.cpp > >> source/Commands/CommandObjectType.cpp > >> source/Commands/CommandObjectWatchpoint.cpp > >> source/Commands/CommandObjectWatchpointCommand.cpp > >> source/Interpreter/OptionGroupArchitecture.cpp > >> source/Interpreter/OptionGroupFormat.cpp > >> source/Interpreter/OptionGroupOutputFile.cpp > >> source/Interpreter/OptionGroupPlatform.cpp > >> source/Interpreter/OptionGroupUUID.cpp > >> source/Interpreter/OptionGroupValueObjectDisplay.cpp > >> source/Interpreter/OptionGroupVariable.cpp > >> source/Interpreter/OptionGroupWatchpoint.cpp > >> source/Interpreter/Options.cpp > >> source/Target/Platform.cpp > >> source/Target/Process.cpp > >> <D4369.11043.patch>_______________________________________________ > >> lldb-commits mailing list > >> [email protected] > >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > _______________________________________________ > > lldb-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > _______________________________________________ > lldb-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
