[lldb] Add method decls to a CXXRecordDecl only after all their properties are defined

[teemperor]

Summary

Calling addDecl on a CXXRecordDecl is not a trivial method but is actually inspecting the added
declarations to infer properties about the CXXRecordDecl. Whatever declaration we pass
to addDecl should be in its final state so we should first set all the properties of such a decl
and then call addDecl. If we do it the other way around like we do here then addDecl may
do incorrect decisions.

The only code that is currently after addDecl is changing whether the special members are
defaulted/trivial. I'm not sure if this actually fixes anything but it's more correct than what we
did before.

[kwk]

The only code that is currently after addDecl is changing whether the special members are
defaulted/trivial. I'm not sure if this actually fixes anything but it's more correct than what we
did before.

But at least you return immediately after calling addDecl.

When trying to see what VerifyDecl(cxx_method_decl) does, I noticed that I don't have a ClangASTContext.cpp in my source tree. Even grepping for the file in the last 400 revisions didn't show it: git log --stat -n400 | grep ClangASTContext.cpp. No luck with github as well: https://github.com/llvm/llvm-project/search?q=ClangASTContext.cpp&unscoped_q=ClangASTContext.cpp. Is that expected?

lldb/source/Symbol/ClangASTContext.cpp

params.push_back(clang::ParmVarDecl::Create(
         clang::SourceLocation(),
         nullptr, // anonymous
         method_function_prototype->getParamType(param_index), nullptr,
         clang::SC_None, nullptr));
   }
 
   cxx_method_decl->setParams(llvm::ArrayRef<clang::ParmVarDecl *>(params));
 
-  cxx_record_decl->addDecl(cxx_method_decl);
 
   // Sometimes the debug info will mention a constructor (default/copy/move),
   // destructor, or assignment operator (copy/move) but there won't be any
   // version of this in the code. So we check if the function was artificially
   // generated and if it is trivial and this lets the compiler/backend know
   // that it can inline the IR for these when it needs to and we can avoid a
   // "missing function" error when running expressions.
 
if (cxx_ctor_decl && ((cxx_ctor_decl->isDefaultConstructor() &&
       cxx_method_decl->setTrivial(true);
     }
   }
 
 #ifdef LLDB_CONFIGURATION_DEBUG
   VerifyDecl(cxx_method_decl);
 #endif
 
+  cxx_record_decl->addDecl(cxx_method_decl);
+
   return cxx_method_decl;
 }
 
 void ClangASTContext::AddMethodOverridesForCXXRecordType(
     lldb::opaque_compiler_type_t type) {
   if (auto *record = GetAsCXXRecordDecl(type))
     for (auto *method : record->methods())
       addOverridesForMethod(method);