Refactored the clang completer; many bugs fixed

This change should fix the random hangs and segfaults when using the clang
completer. Also, assertion errors printed to the console on vim exit should go
away too, same thing with segfaults on vim exit. These "on exit" errors were
caused by not cleanly shutting down the background threads; both the identifier
completer and the clang one now join the threads on destruction. This results in
a clean shutdown.

The new clang completer architecture now uses only one clang thread (again)
instead of a completion and parsing thread. Since the parsing task needs to wait
on the completion task if it was started first (and vice-versa) there's no point
to using two threads. The desired "simplicity" of using two threads for these
two tasks actually created needless complexity (and bugs). Sigh. Such is life.

A TranslationUnit abstraction was also created and this in turn also reduces the
complexity of the clang completer.

The clang completer now also has some (very) basic tests.

autoload/youcompleteme.vim

@@ -208,6 +208,11 @@ function! s:InvokeCompletion()
   " So we solve this with the searched_and_no_results_found script-scope
   " variable that prevents this infinite loop from starting.
   if pumvisible() || s:searched_and_no_results_found
+    " TODO: try a different approach where after we return some completions to
+    " Vim we don't trigger the feedkeys call UNLESS the user has moved in
+    " insert/normal mode; this could help with that insidious and impossible to
+    " reproduce completion-blocking-typing bug; we could implement this by
+    " storing the last line & column
     let s:searched_and_no_results_found = 0
     return
   endif
@@ -237,7 +242,7 @@ function! s:CompletionsForQuery( query, use_filetype_completer )
   while !l:results_ready
     let l:results_ready = pyeval( 'completer.AsyncCandidateRequestReady()' )
     if complete_check()
-      let s:searched_and_no_results_found = 0
+      let s:searched_and_no_results_found = 1
       return { 'words' : [], 'refresh' : 'always'}
     endif
   endwhile

cpp/ycm/ClangCompleter.cpp

@@ -17,6 +17,7 @@
 
 #include "ClangCompleter.h"
 #include "Candidate.h"
+#include "TranslationUnit.h"
 #include "CompletionData.h"
 #include "standard.h"
 #include "CandidateRepository.h"
@@ -39,7 +40,9 @@ using boost::bind;
 using boost::thread;
 using boost::lock_guard;
 using boost::unique_lock;
+using boost::shared_lock;
 using boost::mutex;
+using boost::shared_mutex;
 using boost::unordered_map;
 using boost::try_to_lock_t;
 
@@ -77,6 +80,7 @@ struct CompletionDataAndResult
 ClangCompleter::ClangCompleter()
   : candidate_repository_( CandidateRepository::Instance() ),
     threading_enabled_( false ),
+    time_to_die_( false ),
     clang_data_ready_( false )
 {
   clang_index_ = clang_createIndex( 0, 0 );
@@ -85,13 +89,21 @@ ClangCompleter::ClangCompleter()
 
 ClangCompleter::~ClangCompleter()
 {
-  foreach ( const TranslationUnitForFilename::value_type &filename_unit,
+  // We need to clear this before calling clang_disposeIndex because the
-            filename_to_translation_unit_ )
+  // translation units need to be destroyed before the index is destroyed.
+  filename_to_translation_unit_.clear();
+  clang_disposeIndex( clang_index_ );
+
   {
-    clang_disposeTranslationUnit( filename_unit.second );
+    unique_lock< shared_mutex > lock( time_to_die_mutex_ );
+    time_to_die_ = true;
   }
 
-  clang_disposeIndex( clang_index_ );
+  sorting_threads_.interrupt_all();
+  sorting_threads_.join_all();
+
+  clang_thread_.interrupt();
+  clang_thread_.join();
 }
 
 
@@ -107,37 +119,37 @@ void ClangCompleter::EnableThreading()
 std::vector< Diagnostic > ClangCompleter::DiagnosticsForFile(
     const std::string &filename )
 {
-  std::vector< Diagnostic > diagnostics;
+  shared_ptr< TranslationUnit > unit;
-  unique_lock< mutex > lock( clang_access_mutex_, try_to_lock_t() );
-  if ( !lock.owns_lock() )
-    return diagnostics;
-
-  CXTranslationUnit unit = FindWithDefault( filename_to_translation_unit_,
-                                            filename,
-                                            NULL );
-  if ( !unit )
-    return diagnostics;
-
-  uint num_diagnostics = clang_getNumDiagnostics( unit );
-  diagnostics.reserve( num_diagnostics );
-
-  for ( uint i = 0; i < num_diagnostics; ++i )
   {
-    Diagnostic diagnostic = CXDiagnosticToDiagnostic(
+    lock_guard< mutex > lock( filename_to_translation_unit_mutex_ );
-        clang_getDiagnostic( unit, i ) );
+    unit = FindWithDefault(
-
+        filename_to_translation_unit_,
-    if ( diagnostic.kind_ != 'I' )
+        filename,
-      diagnostics.push_back( diagnostic );
+        shared_ptr< TranslationUnit >() );
   }
 
-  return diagnostics;
+  if ( !unit )
+    return std::vector< Diagnostic >();
+
+  return unit->LatestDiagnostics();
 }
 
 
-bool ClangCompleter::UpdatingTranslationUnit()
+bool ClangCompleter::UpdatingTranslationUnit( const std::string &filename )
 {
-  unique_lock< mutex > lock( clang_access_mutex_, try_to_lock_t() );
+  shared_ptr< TranslationUnit > unit;
-  return !lock.owns_lock();
+  {
+    lock_guard< mutex > lock( filename_to_translation_unit_mutex_ );
+    unit = FindWithDefault(
+        filename_to_translation_unit_,
+        filename,
+        shared_ptr< TranslationUnit >() );
+  }
+
+  if ( !unit )
+    return false;
+
+  return unit->IsCurrentlyUpdating();
 }
 
 
@@ -146,26 +158,12 @@ void ClangCompleter::UpdateTranslationUnit(
     const std::vector< UnsavedFile > &unsaved_files,
     const std::vector< std::string > &flags )
 {
-  TranslationUnitForFilename::iterator it =
+  shared_ptr< TranslationUnit > unit = GetTranslationUnitForFile( filename,
-    filename_to_translation_unit_.find( filename );
+                                                                  unsaved_files,
-
+                                                                  flags );
-  if ( it != filename_to_translation_unit_.end() )
+  X_ASSERT( unit );
-  {
+  // TODO: only do this if the unit was not just created
-    std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
+  unit->Reparse( unsaved_files );
-        unsaved_files );
-
-    clang_reparseTranslationUnit(
-        it->second,
-        cxunsaved_files.size(),
-        &cxunsaved_files[ 0 ],
-        clang_defaultEditingTranslationUnitOptions() );
-  }
-
-  else
-  {
-    filename_to_translation_unit_[ filename ] =
-      CreateTranslationUnit( filename, unsaved_files, flags );
-  }
 }
 
 
@@ -181,52 +179,37 @@ void ClangCompleter::UpdateTranslationUnitAsync(
           boost::move( unsaved_files ),
           boost::move( flags ) );
 
-  boost::lock_guard< boost::mutex > lock( file_parse_task_mutex_ );
+  // boost::lock_guard< boost::mutex > lock( file_parse_task_mutex_ );
 
-  // Only ever set the task when it's NULL; if it's not, that means that the
+  // // Only ever set the task when it's NULL; if it's not, that means that the
-  // clang thread is working on it
+  // // clang thread is working on it
-  if ( file_parse_task_ )
+  // if ( file_parse_task_ )
-    return;
+  //   return;
 
-  file_parse_task_ = make_shared< packaged_task< void > >( functor );
+  shared_ptr< ClangPackagedTask > clang_packaged_task =
-  file_parse_task_condition_variable_.notify_all();
+    make_shared< ClangPackagedTask >();
+  clang_packaged_task->parsing_task_ = packaged_task< void >( functor );
+  clang_task_.Set( clang_packaged_task );
+  // file_parse_task_ = make_shared< packaged_task< void > >( functor );
+  // file_parse_task_condition_variable_.notify_all();
 }
 
 
-std::vector< CompletionData > ClangCompleter::CandidatesForLocationInFile(
+std::vector< CompletionData >
+ClangCompleter::CandidatesForLocationInFile(
     const std::string &filename,
     int line,
     int column,
     const std::vector< UnsavedFile > &unsaved_files,
     const std::vector< std::string > &flags )
 {
-  std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
+  shared_ptr< TranslationUnit > unit = GetTranslationUnitForFile( filename,
-      unsaved_files );
+                                                                  unsaved_files,
-
+                                                                  flags );
-  // codeCompleteAt reparses the TU if the underlying source file has changed on
+  X_ASSERT( unit );
-  // disk since the last time the TU was updated and there are no unsaved files.
+  return unit->CandidatesForLocation( line,
-  // If there are unsaved files, then codeCompleteAt will parse the in-memory
+                                      column,
-  // file contents we are giving it. In short, it is NEVER a good idea to call
+                                      unsaved_files );
-  // clang_reparseTranslationUnit right before a call to clang_codeCompleteAt.
-  // This only makes clang reparse the whole file TWICE, which has a huge impact
-  // on latency. At the time of writing, it seems that most users of libclang
-  // in the open-source world don't realize this (I checked). Some don't even
-  // call reparse*, but parse* which is even less efficient.
-
-  CXCodeCompleteResults *results =
-    clang_codeCompleteAt( GetTranslationUnitForFile( filename,
-                                                     unsaved_files,
-                                                     flags ),
-                          filename.c_str(),
-                          line,
-                          column,
-                          &cxunsaved_files[ 0 ],
-                          cxunsaved_files.size(),
-                          clang_defaultCodeCompleteOptions() );
-
-  std::vector< CompletionData > candidates = ToCompletionDataVector( results );
-  clang_disposeCodeCompleteResults( results );
-  return candidates;
 }
 
 
@@ -246,7 +229,7 @@ ClangCompleter::CandidatesForQueryAndLocationInFileAsync(
   if ( query.empty() )
   {
     // The clang thread is busy, return nothing
-    if ( UpdatingTranslationUnit() )
+    if ( UpdatingTranslationUnit( filename ) )
       return Future< AsyncCompletions >();
 
     {
@@ -281,7 +264,7 @@ ClangCompleter::CandidatesForQueryAndLocationInFileAsync(
   if ( query.empty() )
   {
     FunctionReturnsCompletionDataVector
-      candidates_for_location_in_file_functor =
+      candidates_for_location_functor =
       bind( &ClangCompleter::CandidatesForLocationInFile,
             boost::ref( *this ),
             boost::move( filename ),
@@ -290,74 +273,56 @@ ClangCompleter::CandidatesForQueryAndLocationInFileAsync(
             boost::move( unsaved_files ),
             boost::move( flags ) );
 
-    shared_ptr< packaged_task< AsyncCompletions > > task =
+    shared_ptr< ClangPackagedTask > clang_packaged_task =
-      make_shared< packaged_task< AsyncCompletions > >(
+        make_shared< ClangPackagedTask >();
+    clang_packaged_task->completions_task_ = packaged_task< AsyncCompletions >(
         bind( ReturnValueAsShared< std::vector< CompletionData > >,
-              candidates_for_location_in_file_functor ) );
+              candidates_for_location_functor ) );
 
-    clang_completions_task_.Set( task );
+    // shared_ptr< packaged_task< AsyncCompletions > > task =
+    //   make_shared< packaged_task< AsyncCompletions > >(
+    //     bind( ReturnValueAsShared< std::vector< CompletionData > >,
+    //           candidates_for_location_functor ) );
+
+    clang_task_.Set( clang_packaged_task );
   }
 
   return Future< AsyncCompletions >( boost::move( future ) );
 }
 
 
-CXTranslationUnit ClangCompleter::CreateTranslationUnit(
+// WARNING: It should not be possible to call this function from two separate
+// threads at the same time. Currently only one thread (the clang thread) ever
+// calls this function so there is no need for a mutex, but if that changes in
+// the future a mutex will be needed to make sure that two threads don't try to
+// create the same translation unit.
+boost::shared_ptr< TranslationUnit > ClangCompleter::GetTranslationUnitForFile(
     const std::string &filename,
     const std::vector< UnsavedFile > &unsaved_files,
     const std::vector< std::string > &flags )
 {
-  std::vector< const char* > pointer_flags;
-  pointer_flags.reserve( flags.size() );
-
-  foreach ( const std::string &flag, flags )
   {
-    pointer_flags.push_back( flag.c_str() );
+    lock_guard< mutex > lock( filename_to_translation_unit_mutex_ );
+    TranslationUnitForFilename::iterator it =
+      filename_to_translation_unit_.find( filename );
+
+    if ( it != filename_to_translation_unit_.end() )
+      return it->second;
   }
 
-  std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
-      unsaved_files );
 
-  CXTranslationUnit unit = clang_parseTranslationUnit(
+  shared_ptr< TranslationUnit > unit = make_shared< TranslationUnit >(
-      clang_index_,
+      filename, unsaved_files, flags, clang_index_ );
-      filename.c_str(),
-      &pointer_flags[ 0 ],
-      pointer_flags.size(),
-      &cxunsaved_files[ 0 ],
-      cxunsaved_files.size(),
-      clang_defaultEditingTranslationUnitOptions() );
 
-  // Only with a reparse is the preable precompiled. I do not know why...
+  {
-  // TODO: report this bug on the clang tracker
+    lock_guard< mutex > lock( filename_to_translation_unit_mutex_ );
-  clang_reparseTranslationUnit(
+    filename_to_translation_unit_[ filename ] = unit;
-      unit,
+  }
-      cxunsaved_files.size(),
-      &cxunsaved_files[ 0 ],
-      clang_defaultEditingTranslationUnitOptions() );
 
   return unit;
 }
 
 
-CXTranslationUnit ClangCompleter::GetTranslationUnitForFile(
-    const std::string &filename,
-    const std::vector< UnsavedFile > &unsaved_files,
-    const std::vector< std::string > &flags )
-{
-  TranslationUnitForFilename::iterator it =
-    filename_to_translation_unit_.find( filename );
-
-  if ( it != filename_to_translation_unit_.end() )
-    return it->second;
-
-  CXTranslationUnit unit = CreateTranslationUnit( filename,
-                                                  unsaved_files,
-                                                  flags );
-  filename_to_translation_unit_[ filename ] = unit;
-  return unit;
-}
-
-
 std::vector< CompletionData > ClangCompleter::SortCandidatesForQuery(
     const std::string &query,
     const std::vector< CompletionData > &completion_datas )
@@ -410,75 +375,86 @@ void ClangCompleter::InitThreads()
               boost::ref( *this ) ) );
   }
 
-  clang_completions_thread_ = boost::thread(
+  clang_thread_ = boost::thread(
-      &ClangCompleter::ClangCompletionsThreadMain,
+      &ClangCompleter::ClangThreadMain,
-      boost::ref( *this ) );
-
-  file_parse_thread_ = boost::thread(
-      &ClangCompleter::FileParseThreadMain,
       boost::ref( *this ) );
 }
 
 
-void ClangCompleter::FileParseThreadMain()
+// void ClangCompleter::FileParseThreadMain()
+// {
+//   while ( true )
+//   {
+//     {
+//       boost::unique_lock< boost::mutex > lock( file_parse_task_mutex_ );
+//
+//       while ( !file_parse_task_ )
+//       {
+//         file_parse_task_condition_variable_.wait( lock );
+//       }
+//     }
+//
+//     {
+//       unique_lock< mutex > lock( clang_access_mutex_ );
+//       ( *file_parse_task_ )();
+//     }
+//
+//     lock_guard< mutex > lock( file_parse_task_mutex_ );
+//     file_parse_task_ = VoidTask();
+//   }
+// }
+
+
+void ClangCompleter::ClangThreadMain()
 {
   while ( true )
   {
+    try
     {
-      boost::unique_lock< boost::mutex > lock( file_parse_task_mutex_ );
+      // TODO: this should be a separate func, much like the file_parse_task_ part
+      shared_ptr< ClangPackagedTask > task = clang_task_.Get();
 
-      while ( !file_parse_task_ )
+      // If the file parse thread is accessing clang by parsing a file, then drop
-      {
+      // the current completion request
-        file_parse_task_condition_variable_.wait( lock );
+      // {
-      }
+      //   lock_guard< mutex > lock( file_parse_task_mutex_ );
-    }
+      //   if ( file_parse_task_ )
+      //     continue;
+      // }
 
-    {
+      bool has_completions_task = task->completions_task_.valid();
-      unique_lock< mutex > lock( clang_access_mutex_ );
+      if ( has_completions_task )
-      ( *file_parse_task_ )();
+        task->completions_task_();
-    }
+      else
+        task->parsing_task_();
 
-    lock_guard< mutex > lock( file_parse_task_mutex_ );
+      if ( !has_completions_task )
-    file_parse_task_ = VoidTask();
-  }
-}
-
-
-void ClangCompleter::ClangCompletionsThreadMain()
-{
-  while ( true )
-  {
-    // TODO: this should be a separate func, much like the file_parse_task_ part
-    shared_ptr< packaged_task< AsyncCompletions > > task =
-      clang_completions_task_.Get();
-
-    // If the file parse thread is accessing clang by parsing a file, then drop
-    // the current completion request
-    {
-      lock_guard< mutex > lock( file_parse_task_mutex_ );
-      if ( file_parse_task_ )
         continue;
+
+      unique_future< AsyncCompletions > future =
+        task->completions_task_.get_future();
+
+      {
+        boost::unique_lock< boost::shared_mutex > writer_lock(
+            latest_clang_results_shared_mutex_ );
+        latest_clang_results_ = *future.get();
+      }
+
+      {
+        boost::lock_guard< boost::mutex > lock( clang_data_ready_mutex_ );
+        clang_data_ready_ = true;
+      }
+
+      clang_data_ready_condition_variable_.notify_all();
     }
 
+    catch ( boost::thread_interrupted& )
     {
-      unique_lock< mutex > lock( clang_access_mutex_ );
+      shared_lock< shared_mutex > lock( time_to_die_mutex_ );
-      ( *task )();
+      if ( time_to_die_ )
+        return;
+      // else do nothing and re-enter the loop
     }
-
-    unique_future< AsyncCompletions > future = task->get_future();
-
-    {
-      boost::unique_lock< boost::shared_mutex > writer_lock(
-          latest_clang_results_shared_mutex_ );
-      latest_clang_results_ = *future.get();
-    }
-
-    {
-      boost::lock_guard< boost::mutex > lock( clang_data_ready_mutex_ );
-      clang_data_ready_ = true;
-    }
-
-    clang_data_ready_condition_variable_.notify_all();
   }
 }
 
@@ -490,7 +466,7 @@ void ClangCompleter::SortingThreadMain()
     try
     {
       {
-        boost::unique_lock< boost::mutex > lock( clang_data_ready_mutex_ );
+        unique_lock< mutex > lock( clang_data_ready_mutex_ );
 
         while ( !clang_data_ready_ )
         {
@@ -511,7 +487,10 @@ void ClangCompleter::SortingThreadMain()
 
     catch ( boost::thread_interrupted& )
     {
-      // Do nothing and re-enter the loop
+      shared_lock< shared_mutex > lock( time_to_die_mutex_ );
+      if ( time_to_die_ )
+        return;
+      // else do nothing and re-enter the loop
     }
   }
 }

cpp/ycm/ClangCompleter.h

@@ -35,6 +35,7 @@ namespace YouCompleteMe
 {
 
 class CandidateRepository;
+class TranslationUnit;
 struct CompletionData;
 
 typedef boost::shared_ptr< std::vector< CompletionData > > AsyncCompletions;
@@ -43,8 +44,8 @@ typedef boost::unordered_map< std::string,
           boost::shared_ptr<
             std::vector< std::string > > > FlagsForFile;
 
-typedef boost::unordered_map< std::string, CXTranslationUnit >
+typedef boost::unordered_map< std::string,
-  TranslationUnitForFilename;
+          boost::shared_ptr< TranslationUnit > > TranslationUnitForFilename;
 
 
 // TODO: document that all filename parameters must be absolute paths
@@ -58,11 +59,13 @@ public:
 
   std::vector< Diagnostic > DiagnosticsForFile( const std::string &filename );
 
-  bool UpdatingTranslationUnit();
+  bool UpdatingTranslationUnit( const std::string &filename );
 
-  void UpdateTranslationUnit( const std::string &filename,
+  // Public because of unit tests (gtest is not very thread-friendly)
-                              const std::vector< UnsavedFile > &unsaved_files,
+  void UpdateTranslationUnit(
-                              const std::vector< std::string > &flags );
+      const std::string &filename,
+      const std::vector< UnsavedFile > &unsaved_files,
+      const std::vector< std::string > &flags );
 
   // NOTE: params are taken by value on purpose! With a C++11 compiler we can
   // avoid internal copies if params are taken by value (move ctors FTW)
@@ -71,6 +74,7 @@ public:
       std::vector< UnsavedFile > unsaved_files,
       std::vector< std::string > flags );
 
+  // Public because of unit tests (gtest is not very thread-friendly)
   std::vector< CompletionData > CandidatesForLocationInFile(
       const std::string &filename,
       int line,
@@ -89,17 +93,23 @@ public:
       std::vector< std::string > flags );
 
 private:
+
+  // This is basically a union. Only one of the two tasks is set to something
+  // valid, the other task is invalid. Which one is valid depends on the caller.
+  struct ClangPackagedTask
+  {
+    boost::packaged_task< AsyncCompletions > completions_task_;
+    boost::packaged_task< void > parsing_task_;
+  };
+
   typedef ConcurrentLatestValue<
             boost::shared_ptr<
-              boost::packaged_task< AsyncCompletions > > > LatestTask;
+              boost::packaged_task< AsyncCompletions > > > LatestSortingTask;
 
-  // caller takes ownership of translation unit
+  typedef ConcurrentLatestValue<
-  CXTranslationUnit CreateTranslationUnit(
+            boost::shared_ptr< ClangPackagedTask > > LatestClangTask;
-      const std::string &filename,
-      const std::vector< UnsavedFile > &unsaved_files,
-      const std::vector< std::string > &flags );
 
-  CXTranslationUnit GetTranslationUnitForFile(
+  boost::shared_ptr< TranslationUnit > GetTranslationUnitForFile(
       const std::string &filename,
       const std::vector< UnsavedFile > &unsaved_files,
       const std::vector< std::string > &flags );
@@ -110,9 +120,7 @@ private:
 
   void InitThreads();
 
-  void FileParseThreadMain();
+  void ClangThreadMain();
-
-  void ClangCompletionsThreadMain();
 
   void SortingThreadMain();
 
@@ -124,29 +132,15 @@ private:
   CXIndex clang_index_;
 
   TranslationUnitForFilename filename_to_translation_unit_;
+  boost::mutex filename_to_translation_unit_mutex_;
 
   CandidateRepository &candidate_repository_;
 
-  // TODO: move everything thread-related into a separated helper class
-  mutable LatestTask clang_completions_task_;
-
-  mutable LatestTask sorting_task_;
-
-  // TODO: wrap the entire clang API with an internal class that then uses this
-  // mutex... actually a TU class with an internal mutex
-  // Only the thread that is holding this mutex can access clang functions
-  boost::mutex clang_access_mutex_;
-
-  // Only the clang thread can make this NULL and only the GUI thread can make
-  // it non-NULL. The file_parse_task_mutex_ is used before checking the state
-  // of NULL. [ NULL for a shared_ptr naturally means default-constructed
-  // shared_ptr]
-  VoidTask file_parse_task_;
-  boost::mutex file_parse_task_mutex_;
-  boost::condition_variable file_parse_task_condition_variable_;
-
   bool threading_enabled_;
 
+  bool time_to_die_;
+  boost::shared_mutex time_to_die_mutex_;
+
   // TODO: use boost.atomic for clang_data_ready_
   bool clang_data_ready_;
   boost::mutex clang_data_ready_mutex_;
@@ -158,11 +152,13 @@ private:
   // Unfortunately clang is not thread-safe so we need to be careful when we
   // access it. Only one thread at a time is allowed to access any single
   // translation unit.
-  boost::thread clang_completions_thread_;
+  boost::thread clang_thread_;
-
-  boost::thread file_parse_thread_;
 
   boost::thread_group sorting_threads_;
+
+  mutable LatestClangTask clang_task_;
+
+  mutable LatestSortingTask sorting_task_;
 };
 
 } // namespace YouCompleteMe

cpp/ycm/Future.h

@@ -58,6 +58,12 @@ public:
   }
 
 
+  void Wait()
+  {
+    future_.wait();
+  }
+
+
   T GetResults()
   {
     try

cpp/ycm/IdentifierCompleter.cpp

@@ -48,7 +48,14 @@ void QueryThreadMain( LatestQueryTask &latest_query_task )
 {
   while ( true )
   {
-    ( *latest_query_task.Get() )();
+    try
+    {
+      ( *latest_query_task.Get() )();
+    }
+    catch ( boost::thread_interrupted& )
+    {
+      return;
+    }
   }
 
 }
@@ -58,7 +65,14 @@ void BufferIdentifiersThreadMain(
 {
   while ( true )
   {
-    ( *buffer_identifiers_task_stack.Pop() )();
+    try
+    {
+      ( *buffer_identifiers_task_stack.Pop() )();
+    }
+    catch ( boost::thread_interrupted& )
+    {
+      return;
+    }
   }
 }
 
@@ -93,6 +107,16 @@ IdentifierCompleter::IdentifierCompleter(
 }
 
 
+IdentifierCompleter::~IdentifierCompleter()
+{
+  query_threads_.interrupt_all();
+  query_threads_.join_all();
+
+  buffer_identifiers_thread_.interrupt();
+  buffer_identifiers_thread_.join();
+}
+
+
 // We need this mostly so that we can not use it in tests. Apparently the
 // GoogleTest framework goes apeshit on us if we enable threads by default.
 void IdentifierCompleter::EnableThreading()

cpp/ycm/IdentifierCompleter.h

@@ -70,6 +70,8 @@ public:
              const std::string &filetype,
              const std::string &filepath );
 
+  ~IdentifierCompleter();
+
   void EnableThreading();
 
   void AddCandidatesToDatabase(
@@ -125,15 +127,15 @@ private:
 
   FiletypeMap filetype_map_;
 
-  mutable LatestQueryTask latest_query_task_;
-
-  BufferIdentifiersTaskStack buffer_identifiers_task_stack_;
-
   bool threading_enabled_;
 
   boost::thread_group query_threads_;
 
   boost::thread buffer_identifiers_thread_;
+
+  mutable LatestQueryTask latest_query_task_;
+
+  BufferIdentifiersTaskStack buffer_identifiers_task_stack_;
 };
 
 } // namespace YouCompleteMe

cpp/ycm/TranslationUnit.cpp

@@ -0,0 +1,175 @@
+// Copyright (C) 2011, 2012  Strahinja Val Markovic  <val@markovic.io>
+//
+// This file is part of YouCompleteMe.
+//
+// YouCompleteMe is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// YouCompleteMe is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
+
+#include "TranslationUnit.h"
+#include "CompletionData.h"
+#include "standard.h"
+#include "ClangUtils.h"
+
+#include <clang-c/Index.h>
+#include <boost/make_shared.hpp>
+
+using boost::unique_lock;
+using boost::mutex;
+using boost::try_to_lock_t;
+
+namespace YouCompleteMe
+{
+
+TranslationUnit::TranslationUnit(
+    const std::string &filename,
+    const std::vector< UnsavedFile > &unsaved_files,
+    const std::vector< std::string > &flags,
+    CXIndex clang_index )
+  : filename_( filename )
+{
+  std::vector< const char* > pointer_flags;
+  pointer_flags.reserve( flags.size() );
+
+  foreach ( const std::string &flag, flags )
+  {
+    pointer_flags.push_back( flag.c_str() );
+  }
+
+  std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
+      unsaved_files );
+
+  clang_translation_unit_ = clang_parseTranslationUnit(
+      clang_index,
+      filename.c_str(),
+      &pointer_flags[ 0 ],
+      pointer_flags.size(),
+      &cxunsaved_files[ 0 ],
+      cxunsaved_files.size(),
+      clang_defaultEditingTranslationUnitOptions() );
+
+  // Only with a reparse is the preable precompiled. I do not know why...
+  // TODO: report this bug on the clang tracker
+  clang_reparseTranslationUnit(
+      clang_translation_unit_,
+      cxunsaved_files.size(),
+      &cxunsaved_files[ 0 ],
+      clang_defaultEditingTranslationUnitOptions() );
+}
+
+
+TranslationUnit::~TranslationUnit()
+{
+  clang_disposeTranslationUnit( clang_translation_unit_ );
+}
+
+
+std::vector< Diagnostic > TranslationUnit::LatestDiagnostics()
+{
+  std::vector< Diagnostic > diagnostics;
+  unique_lock< mutex > lock( diagnostics_mutex_ );
+
+  // We don't need the latest diags after we return them once so we swap the
+  // internal data with a new, empty diag vector. This vector is then returned
+  // and on C++11 compilers a move ctor is invoked, thus no copy is created.
+  // Theoretically, just returning the value of a
+  // [boost::|std::]move(latest_diagnostics_) call _should_ leave the
+  // latest_diagnostics_ vector in an emtpy, valid state but I'm not going to
+  // rely on that. I just had to look this up in the standard to be sure, and
+  // future readers of this code (myself included) should not be forced to do
+  // that to understand what the hell is going on.
+
+  std::swap( latest_diagnostics_, diagnostics );
+  return diagnostics;
+}
+
+
+bool TranslationUnit::IsCurrentlyUpdating() const
+{
+  unique_lock< mutex > lock( clang_access_mutex_, try_to_lock_t() );
+  return !lock.owns_lock();
+}
+
+
+void TranslationUnit::Reparse( const std::vector< UnsavedFile > &unsaved_files )
+{
+  unique_lock< mutex > lock( clang_access_mutex_ );
+
+  std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
+      unsaved_files );
+
+  clang_reparseTranslationUnit(
+      clang_translation_unit_,
+      cxunsaved_files.size(),
+      &cxunsaved_files[ 0 ],
+      clang_defaultEditingTranslationUnitOptions() );
+
+  UpdateLatestDiagnostics();
+}
+
+
+std::vector< CompletionData > TranslationUnit::CandidatesForLocation(
+    int line,
+    int column,
+    const std::vector< UnsavedFile > &unsaved_files )
+{
+  unique_lock< mutex > lock( clang_access_mutex_ );
+
+  std::vector< CXUnsavedFile > cxunsaved_files = ToCXUnsavedFiles(
+      unsaved_files );
+
+  // codeCompleteAt reparses the TU if the underlying source file has changed on
+  // disk since the last time the TU was updated and there are no unsaved files.
+  // If there are unsaved files, then codeCompleteAt will parse the in-memory
+  // file contents we are giving it. In short, it is NEVER a good idea to call
+  // clang_reparseTranslationUnit right before a call to clang_codeCompleteAt.
+  // This only makes clang reparse the whole file TWICE, which has a huge impact
+  // on latency. At the time of writing, it seems that most users of libclang
+  // in the open-source world don't realize this (I checked). Some don't even
+  // call reparse*, but parse* which is even less efficient.
+
+  CXCodeCompleteResults *results =
+    clang_codeCompleteAt( clang_translation_unit_,
+                          filename_.c_str(),
+                          line,
+                          column,
+                          &cxunsaved_files[ 0 ],
+                          cxunsaved_files.size(),
+                          clang_defaultCodeCompleteOptions() );
+
+  std::vector< CompletionData > candidates = ToCompletionDataVector( results );
+  clang_disposeCodeCompleteResults( results );
+  return candidates;
+}
+
+
+// Should only be called while holding the clang_access_mutex_
+// TODO: assert that
+void TranslationUnit::UpdateLatestDiagnostics()
+{
+  unique_lock< mutex > lock( diagnostics_mutex_ );
+
+  latest_diagnostics_.clear();
+  uint num_diagnostics = clang_getNumDiagnostics( clang_translation_unit_ );
+  latest_diagnostics_.reserve( num_diagnostics );
+
+  for ( uint i = 0; i < num_diagnostics; ++i )
+  {
+    Diagnostic diagnostic = CXDiagnosticToDiagnostic(
+        clang_getDiagnostic( clang_translation_unit_, i ) );
+
+    if ( diagnostic.kind_ != 'I' )
+      latest_diagnostics_.push_back( diagnostic );
+  }
+}
+
+} // namespace YouCompleteMe

cpp/ycm/TranslationUnit.h

@@ -0,0 +1,85 @@
+// Copyright (C) 2011, 2012  Strahinja Val Markovic  <val@markovic.io>
+//
+// This file is part of YouCompleteMe.
+//
+// YouCompleteMe is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// YouCompleteMe is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
+
+#ifndef TRANSLATIONUNIT_H_XQ7I6SVA
+#define TRANSLATIONUNIT_H_XQ7I6SVA
+
+#include "ConcurrentLatestValue.h"
+#include "Future.h"
+#include "UnsavedFile.h"
+#include "Diagnostic.h"
+
+#include <boost/utility.hpp>
+#include <boost/thread/mutex.hpp>
+// #include <boost/unordered_map.hpp>
+
+#include <string>
+#include <vector>
+
+typedef void *CXIndex;
+typedef struct CXTranslationUnitImpl *CXTranslationUnit;
+
+namespace YouCompleteMe
+{
+
+struct CompletionData;
+typedef boost::shared_ptr< std::vector< CompletionData > > AsyncCompletions;
+
+class TranslationUnit : boost::noncopyable
+{
+public:
+
+  TranslationUnit(
+    const std::string &filename,
+    const std::vector< UnsavedFile > &unsaved_files,
+    const std::vector< std::string > &flags,
+    CXIndex clang_index );
+
+  ~TranslationUnit();
+
+  std::vector< Diagnostic > LatestDiagnostics();
+
+  bool IsCurrentlyUpdating() const;
+
+  void Reparse( const std::vector< UnsavedFile > &unsaved_files );
+
+  std::vector< CompletionData > CandidatesForLocation(
+      int line,
+      int column,
+      const std::vector< UnsavedFile > &unsaved_files );
+
+private:
+
+  void UpdateLatestDiagnostics();
+
+  /////////////////////////////
+  // PRIVATE MEMBER VARIABLES
+  /////////////////////////////
+
+  std::string filename_;
+
+  boost::mutex diagnostics_mutex_;
+  std::vector< Diagnostic > latest_diagnostics_;
+
+  mutable boost::mutex clang_access_mutex_;
+  CXTranslationUnit clang_translation_unit_;
+};
+
+} // namespace YouCompleteMe
+
+#endif /* end of include guard: TRANSLATIONUNIT_H_XQ7I6SVA */
+

cpp/ycm/tests/CMakeLists.txt

@@ -51,6 +51,14 @@ add_executable( ${PROJECT_NAME}
 
 target_link_libraries( ${PROJECT_NAME}
                        ycm_core
-                       gmock_main
+                       gmock_main )
-                     )
 
+# The test executable expects a "testdata" dir in its working directory. Why?
+# Because there's NO reliable, cross-platform way of getting the directory in
+# which the executable is located.
+add_custom_target( copy_testdata
+                   COMMAND cmake -E copy_directory
+                   ${CMAKE_CURRENT_SOURCE_DIR}/testdata
+                   ${CMAKE_CURRENT_BINARY_DIR}/testdata )
+
+add_dependencies( ${PROJECT_NAME} copy_testdata )

cpp/ycm/tests/ClangCompleter_test.cpp

@@ -0,0 +1,74 @@
+// Copyright (C) 2011, 2012  Strahinja Val Markovic  <val@markovic.io>
+//
+// This file is part of YouCompleteMe.
+//
+// YouCompleteMe is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// YouCompleteMe is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
+
+#include "ClangCompleter.h"
+#include "CompletionData.h"
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <boost/filesystem.hpp>
+namespace fs = boost::filesystem;
+
+using ::testing::ElementsAre;
+using ::testing::WhenSorted;
+
+namespace YouCompleteMe
+{
+
+TEST( ClangCompleterTest, CandidatesForLocationInFile )
+{
+  fs::path path_to_testdata = fs::current_path() / fs::path( "testdata" );
+  fs::path test_file = path_to_testdata / fs::path( "basic.cpp" );
+
+  ClangCompleter completer;
+  std::vector< CompletionData > completions =
+    completer.CandidatesForLocationInFile(
+        test_file.string(),
+        11,
+        7,
+        std::vector< UnsavedFile >(),
+        std::vector< std::string >() );
+
+  EXPECT_TRUE( !completions.empty() );
+}
+
+
+TEST( ClangCompleterTest, CandidatesForQueryAndLocationInFileAsync )
+{
+  fs::path path_to_testdata = fs::current_path() / fs::path( "testdata" );
+  fs::path test_file = path_to_testdata / fs::path( "basic.cpp" );
+
+  ClangCompleter completer;
+  completer.EnableThreading();
+
+  Future< AsyncCompletions > completions_future =
+    completer.CandidatesForQueryAndLocationInFileAsync(
+        "",
+        test_file.string(),
+        11,
+        7,
+        std::vector< UnsavedFile >(),
+        std::vector< std::string >() );
+
+  completions_future.Wait();
+
+  EXPECT_TRUE( !completions_future.GetResults()->empty() );
+}
+
+} // namespace YouCompleteMe
+
+

cpp/ycm/tests/testdata/basic.cpp

@@ -0,0 +1,12 @@
+struct Foo {
+  int x;
+  int y;
+  char c;
+}
+
+int main()
+{
+  Foo foo;
+  // The location after the dot is line 11, col 7
+  foo.
+}

cpp/ycm/ycm_core.cpp

@@ -98,7 +98,6 @@ BOOST_PYTHON_MODULE(ycm_core)
     .def( "EnableThreading", &ClangCompleter::EnableThreading )
     .def( "DiagnosticsForFile", &ClangCompleter::DiagnosticsForFile )
     .def( "UpdatingTranslationUnit", &ClangCompleter::UpdatingTranslationUnit )
-    .def( "UpdateTranslationUnit", &ClangCompleter::UpdateTranslationUnit )
     .def( "UpdateTranslationUnitAsync",
           &ClangCompleter::UpdateTranslationUnitAsync )
     .def( "CandidatesForQueryAndLocationInFileAsync",

python/completers/cpp/clang_completer.py

@@ -66,7 +66,7 @@ class ClangCompleter( Completer ):
 
 
   def CandidatesForQueryAsync( self, query ):
-    if self.completer.UpdatingTranslationUnit():
+    if self.completer.UpdatingTranslationUnit( vim.current.buffer.name ):
       vimsupport.PostVimMessage( 'Still parsing file, no completions yet.' )
       self.future = None
       return
@@ -121,7 +121,7 @@ class ClangCompleter( Completer ):
 
   def DiagnosticsForCurrentFileReady( self ):
     return ( self.possibly_new_diagnostics and not
-             self.completer.UpdatingTranslationUnit() )
+             self.completer.UpdatingTranslationUnit( vim.current.buffer.name ) )
 
 
   def GetDiagnosticsForCurrentFile( self ):
@@ -137,8 +137,6 @@ class ClangCompleter( Completer ):
     return ShouldUseClang( start_column )
 
 
-
-
 def CompletionDataToDict( completion_data ):
   # see :h complete-items for a description of the dictionary fields
   return {