> On Feb 5, 2016, at 5:05 PM, Jim Ingham <jing...@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 <egran...@apple.com> wrote: >> >>> >>> On Feb 5, 2016, at 4:57 PM, Jim Ingham <jing...@apple.com> wrote: >>> >>> >>>> On Feb 5, 2016, at 4:43 PM, Enrico Granata via lldb-commits >>>> <lldb-commits@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
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
