<div dir="ltr">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.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 3, 2014 at 10:43 AM, <span dir="ltr"><<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""><br>
> On Jul 3, 2014, at 10:41 AM, Greg Clayton <<a href="mailto:gclayton@apple.com">gclayton@apple.com</a>> wrote:<br>
><br>
> Looks fine. One question:<br>
><br>
> In the definition of the OptionValidator:<br>
><br>
> struct OptionValidator<br>
> {<br>
> virtual ~OptionValidator() { }<br>
> virtual bool IsValid() const = 0;<br>
> virtual const char * ValidConditionString() const = 0;<br>
> };<br>
><br>
> 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.<br>
<br>
</div>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.<br>
<span class="HOEnZb"><font color="#888888"><br>
Jim<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Greg<br>
><br>
>> On Jul 3, 2014, at 12:33 AM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
>><br>
>> 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.<br>
>><br>
>> 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.<br>
>><br>
>> <a href="http://reviews.llvm.org/D4369" target="_blank">http://reviews.llvm.org/D4369</a><br>
>><br>
>> Files:<br>
>> include/lldb/lldb-private-types.h<br>
>> source/Commands/CommandObjectArgs.cpp<br>
>> source/Commands/CommandObjectBreakpoint.cpp<br>
>> source/Commands/CommandObjectBreakpointCommand.cpp<br>
>> source/Commands/CommandObjectCommands.cpp<br>
>> source/Commands/CommandObjectDisassemble.cpp<br>
>> source/Commands/CommandObjectExpression.cpp<br>
>> source/Commands/CommandObjectFrame.cpp<br>
>> source/Commands/CommandObjectHelp.cpp<br>
>> source/Commands/CommandObjectLog.cpp<br>
>> source/Commands/CommandObjectMemory.cpp<br>
>> source/Commands/CommandObjectPlatform.cpp<br>
>> source/Commands/CommandObjectProcess.cpp<br>
>> source/Commands/CommandObjectRegister.cpp<br>
>> source/Commands/CommandObjectSettings.cpp<br>
>> source/Commands/CommandObjectSource.cpp<br>
>> source/Commands/CommandObjectTarget.cpp<br>
>> source/Commands/CommandObjectThread.cpp<br>
>> source/Commands/CommandObjectType.cpp<br>
>> source/Commands/CommandObjectWatchpoint.cpp<br>
>> source/Commands/CommandObjectWatchpointCommand.cpp<br>
>> source/Interpreter/OptionGroupArchitecture.cpp<br>
>> source/Interpreter/OptionGroupFormat.cpp<br>
>> source/Interpreter/OptionGroupOutputFile.cpp<br>
>> source/Interpreter/OptionGroupPlatform.cpp<br>
>> source/Interpreter/OptionGroupUUID.cpp<br>
>> source/Interpreter/OptionGroupValueObjectDisplay.cpp<br>
>> source/Interpreter/OptionGroupVariable.cpp<br>
>> source/Interpreter/OptionGroupWatchpoint.cpp<br>
>> source/Interpreter/Options.cpp<br>
>> source/Target/Platform.cpp<br>
>> source/Target/Process.cpp<br>
>> <D4369.11043.patch>_______________________________________________<br>
>> lldb-commits mailing list<br>
>> <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><br></div>