[lldb] Make CompleteTagDeclsScope completion order deterministic

[teemperor]

Summary

We iterate over m_decls_to_complete to complete declarations. As
m_decls_to_complete is a set the iteration order can be non-deterministic.
The order is currently only non-deterministic when we have
a large set of decls that need to be completed (i.e. more than 32 decls,
as otherwise the SmallPtrSet is just a linear-searched list).

This doesn't really fix any specific bug or has any really observable
change in behavior as the order in which we import should not influence
any semantics. However the order we create decls/types is now always
deterministic which should make debugging easier.

[shafik]

This is a good change, we do see bugs that are sometimes not totally deterministic so this should hopefully reduce those.

[davide]

LGTM

lldb/source/Symbol/ClangASTImporter.cpp

 namespace {
 /// Completes all imported TagDecls at the end of the scope.
 ///
 /// While in a CompleteTagDeclsScope, every decl that could be completed will
 /// be completed at the end of the scope (including all Decls that are
 /// imported while completing the original Decls).
 class CompleteTagDeclsScope : public ClangASTImporter::NewDeclListener {
   ClangASTImporter::ImporterDelegateSP m_delegate;
-  llvm::SmallPtrSet<NamedDecl *, 32> m_decls_to_complete;
+  llvm::SmallVector<NamedDecl *, 32> m_decls_to_complete;
   llvm::SmallPtrSet<NamedDecl *, 32> m_decls_already_completed;
   clang::ASTContext *m_dst_ctx;
   clang::ASTContext *m_src_ctx;
   ClangASTImporter &importer;
 
 public:
   /// Constructs a CompleteTagDeclsScope.
   /// \param importer The ClangASTImporter that we should observe.
   /// \param dst_ctx The ASTContext to which Decls are imported.
   /// \param src_ctx The ASTContext from which Decls are imported.
   explicit CompleteTagDeclsScope(ClangASTImporter &importer,
                             clang::ASTContext *dst_ctx,
                             clang::ASTContext *src_ctx)
       : m_delegate(importer.GetDelegate(dst_ctx, src_ctx)), m_dst_ctx(dst_ctx),
         m_src_ctx(src_ctx), importer(importer) {
     m_delegate->SetImportListener(this);
   }
 
   virtual ~CompleteTagDeclsScope() {
     ClangASTImporter::ASTContextMetadataSP to_context_md =
         importer.GetContextMetadata(m_dst_ctx);
 
     // Complete all decls we collected until now.
     while (!m_decls_to_complete.empty()) {
-      NamedDecl *decl = *m_decls_to_complete.begin();
-
+      NamedDecl *decl = m_decls_to_complete.pop_back_val();
       m_decls_already_completed.insert(decl);
-      m_decls_to_complete.erase(decl);
 
       // We should only complete decls coming from the source context.
       assert(to_context_md->m_origins[decl].ctx == m_src_ctx);
 
       Decl *original_decl = to_context_md->m_origins[decl].decl;
 
       // Complete the decl now.
       ClangASTContext::GetCompleteDecl(m_src_ctx, original_decl);
void NewDeclImported(clang::Decl *from, clang::Decl *to) override {
     // We don't need to complete injected class name decls.
     if (from_record_decl && from_record_decl->isInjectedClassName())
       return;
 
     NamedDecl *to_named_decl = dyn_cast<NamedDecl>(to);
     // Check if we already completed this type.
     if (m_decls_already_completed.count(to_named_decl) != 0)
       return;
-    m_decls_to_complete.insert(to_named_decl);
+    m_decls_to_complete.push_back(to_named_decl);
   }
 };
 } // namespace
 
 CompilerType ClangASTImporter::DeportType(ClangASTContext &dst,
                                           const CompilerType &src_type) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));