[Lldb-commits] [lldb] r259964 - Fix an issue where certain CommandObjects (or Options thereof) were being created once, bound to a specific CommandInterpreter (and hence a specific Debugger), and then cached for reuse across different Debugger instances

[Enrico Granata via lldb-commits]

> On Feb 5, 2016, at 5:05 PM, Jim Ingham <jingham at apple.com> wrote:
> 
> Is it ever right for a Command to add a subcommand from a different CommandInterpreter?  If it is not (and I am pretty sure it isn't) then this check should be done in LoadSubCommand, where you would check the to-be-added command's interpreter against the added-to's interpreter.  This would immediately catch this error if anybody makes it in the future.  Probably good to do this in AddCommand for the interpreter as well.
> 
> Jim
> 
>> On Feb 5, 2016, at 5:02 PM, Enrico Granata <egranata at apple.com> wrote:
>> 
>>> 
>>> On Feb 5, 2016, at 4:57 PM, Jim Ingham <jingham at apple.com> wrote:
>>> 
>>> 
>>>> On Feb 5, 2016, at 4:43 PM, Enrico Granata via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>>> 
>>>> Modified: lldb/trunk/source/Target/LanguageRuntime.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/LanguageRuntime.cpp?rev=259964&r1=259963&r2=259964&view=diff
>>>> ==============================================================================
>>>> --- lldb/trunk/source/Target/LanguageRuntime.cpp (original)
>>>> +++ lldb/trunk/source/Target/LanguageRuntime.cpp Fri Feb  5 18:43:07 2016
>>>> @@ -336,6 +336,10 @@ LanguageRuntime::InitializeCommands (Com
>>>>           CommandObjectSP command = command_callback(parent->GetCommandInterpreter());
>>>>           if (command)
>>>>           {
>>>> +                // the CommandObject vended by a Language plugin cannot be created once and cached because
>>>> +                // we may create multiple debuggers and need one instance of the command each - the implementing function
>>>> +                // is meant to create a new instance of the command each time it is invoked
>>>> +                assert(&command->GetCommandInterpreter() == &parent->GetCommandInterpreter() && "language plugin returned command for a mismatched CommandInterpreter");
>>> 
>>> Should CommandObject::LoadSubCommand do this check?
>>> 
>>>>               parent->LoadSubCommand(command->GetCommandName(), command);
>>>>           }
>>>>       }
>>> 
>>> Jim
>>> 
>> 
>> You mean the assert? The point of the assert is that the language plugin was creating a command once and caching it and then returning the cached version to a different debugger
>> Hence I am doing the check specifically for those commands.
>> 
>> But maybe I am missing what you’re trying to suggest here.
>> 
>> Thanks,
>> - Enrico
>> 📩 egranata@.com ☎️ 27683
> 

Aha! Yeah, now I see what you’re saying. And, yes, you do make a good point. I can move the assert there alright.

Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160205/26be2f45/attachment.html>