Update the current execution context at the beginning of tab completions

[MrHate]

Summary

Fix a bug that tab completions won't synchronous the current execution context. ( Thanks for Jim's explanation! )

[labath]

Based on Jim's explanation (thanks!) this sounds like the right thing to do. Does this also fix the "frame var" bug that Jim mentioned on the other thread? Could you also add a test case like that?

lldb/source/Interpreter/CommandObject.cpp

271

If we do this, should we also call Cleanup() at the end of the function?

[MrHate]

Thanks labath, for noting me about my missing test and Cleanup()!
For the test, my idea is to modify the original test for frame variable to make it not run frame variable before completion.
For the Cleanup(), I've checked the related code. It seems that Cleanup() is used to clear m_exe_ctx ( by calling its member function Clear() ) and release m_api_locker. From the comment at the beginning of CheckRequirements(), it says "Every command should call CommandObject::Cleanup() after it has completed." and we can find that Cleanup() appears only at the end of Execute(). But from CommandInterpreter::GetExecutionContext, ExecutionContextRef::Lock and ExecutionContext::ExecutionContext(const ExecutionContextRef*, bool) we can know that, this process is just assignments on smart pointers. So I think using m_exe_ctx.Clear() here might be better.

[MrHate]

Modified the test for frame variable so that it will not execute frame variable before completion test.
Added m_exe_ctx.Clear() at the end of CommandObject::HandleCompletion to ensure that nothing will be stored in m_exe_ctx between running commands.

[labath]

In D75597#1905163, @MrHate wrote:

Thanks labath, for noting me about my missing test and Cleanup()!
For the test, my idea is to modify the original test for frame variable to make it not run frame variable before completion.
For the Cleanup(), I've checked the related code. It seems that Cleanup() is used to clear m_exe_ctx ( by calling its member function Clear() ) and release m_api_locker. From the comment at the beginning of CheckRequirements(), it says "Every command should call CommandObject::Cleanup() after it has completed." and we can find that Cleanup() appears only at the end of Execute(). But from CommandInterpreter::GetExecutionContext, ExecutionContextRef::Lock and ExecutionContext::ExecutionContext(const ExecutionContextRef*, bool) we can know that, this process is just assignments on smart pointers. So I think using m_exe_ctx.Clear() here might be better.

Yes, that sounds reasonable.

lldb/test/API/functionalities/completion/TestCompletion.py

46–47

Please also remove this FIXME. Instead, you could add a comment that we're explicitly testing the scenario where "frame var" is completed without any preceding commands.

[MrHate]

Added comments about the new version test for command frame variable.

[jingham]

LGTM, thanks for fixing this.

[teemperor]

I'll land this for @MrHate (he's my GSoC student)

lldb/source/Interpreter/CommandInterpreter.cpp

if (command_object) {
       request.ShiftArguments();
       command_object->HandleCompletion(request);
     }
   }
 }
 
 void CommandInterpreter::HandleCompletion(CompletionRequest &request) {
 
+  UpdateExecutionContext(nullptr);
+
   // Don't complete comments, and if the line we are completing is just the
   // history repeat character, substitute the appropriate history line.
   llvm::StringRef first_arg = request.GetParsedLine().GetArgumentAtIndex(0);
 
   if (!first_arg.empty()) {
     if (first_arg.front() == m_comment_char)
       return;
     if (first_arg.front() == CommandHistory::g_repeat_char) {

lldb/source/Interpreter/CommandObject.cpp

 
 void CommandObject::Cleanup() {
   m_exe_ctx.Clear();
   if (m_api_locker.owns_lock())
     m_api_locker.unlock();
 }
 
 void CommandObject::HandleCompletion(CompletionRequest &request) {
+
+  m_exe_ctx = m_interpreter.GetExecutionContext();
+
   // Default implementation of WantsCompletion() is !WantsRawCommandString().
   // Subclasses who want raw command string but desire, for example, argument
   // completion should override WantsCompletion() to return true, instead.
   if (WantsRawCommandString() && !WantsCompletion()) {
     // FIXME: Abstract telling the completion to insert the completion
     // character.
     return;
   } else {
if (cur_options != nullptr) {
           request, opt_element_vector, GetCommandInterpreter());
       if (handled_by_options)
         return;
     }
 
     // If we got here, the last word is not an option or an option argument.
     HandleArgumentCompletion(request, opt_element_vector);
   }
+
+  m_exe_ctx.Clear();
 }
 
 bool CommandObject::HelpTextContainsWord(llvm::StringRef search_word,
                                          bool search_short_help,
                                          bool search_long_help,
                                          bool search_syntax,
                                          bool search_options) {
   std::string options_usage_help;

lldb/test/API/functionalities/completion/TestCompletion.py

class CommandLineCompletionTestCase(TestBase):
     def test_frame_variable(self):
         self.build()
         self.main_source = "main.cpp"
         self.main_source_spec = lldb.SBFileSpec(self.main_source)
 
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                           '// Break here', self.main_source_spec)
         self.assertEquals(process.GetState(), lldb.eStateStopped)
-        # FIXME: This pulls in the debug information to make the completions work,
-        # but the completions should also work without.
-        self.runCmd("frame variable fooo")
+        
+        # Since CommandInterpreter has been corrected to update the current execution 
+        # context at the beginning of HandleCompletion, we're here explicitly testing  
+        # the scenario where "frame var" is completed without any preceding commands.
 
         self.complete_from_to('frame variable fo',
                               'frame variable fooo')
         self.complete_from_to('frame variable fooo.',
                               'frame variable fooo.')
         self.complete_from_to('frame variable fooo.dd',
                               'frame variable fooo.dd')