[ASTImporter] Remove manual completion of FieldDecls workaround

[teemperor]

Summary

[Note: This is not a standalone review, but a split out part of D101950.
I just split it out to make it easier to review.]

This patch just removes an LLDB-specific workaround from the ASTImporter.
We no longer require any workarounds that manually pull in definitions since
D101950 now does that automatically for us (by actually listening to all the
callbacks we get from Clang towards LLDB's custom ExternalASTSource).

[martong]

Looks good!

I guess you're planning to commit this only if https://reviews.llvm.org/D101950 is accepted?

[teemperor]

In D112132#3075283, @martong wrote:

Looks good!

I guess you're planning to commit this only if https://reviews.llvm.org/D101950 is accepted?

Jepp, the patches where I put the disclaimer at the top I'll commit as one with D101950 (other the commits between those would have broken tests and we don't allow merge commits in LLVM, so that seems to be the only way).

clang/lib/AST/ASTImporter.cpp

ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) {
   // because there is an ODR error with two typedefs.  As another example,
   // the client may allow EnumConstantDecls with same names but with
   // different values in two distinct translation units.
   bool AccumulateChildErrors = isa<TagDecl>(FromDC);
 
   Error ChildErrors = Error::success();
   for (auto *From : FromDC->decls()) {
     ExpectedDecl ImportedOrErr = import(From);
-
-    // If we are in the process of ImportDefinition(...) for a RecordDecl we
-    // want to make sure that we are also completing each FieldDecl. There
-    // are currently cases where this does not happen and this is correctness
-    // fix since operations such as code generation will expect this to be so.
-    if (ImportedOrErr) {
-      FieldDecl *FieldFrom = dyn_cast_or_null<FieldDecl>(From);
-      Decl *ImportedDecl = *ImportedOrErr;
-      FieldDecl *FieldTo = dyn_cast_or_null<FieldDecl>(ImportedDecl);
-      if (FieldFrom && FieldTo) {
-        RecordDecl *FromRecordDecl = nullptr;
-        RecordDecl *ToRecordDecl = nullptr;
-        // If we have a field that is an ArrayType we need to check if the array
-        // element is a RecordDecl and if so we need to import the definition.
-        if (FieldFrom->getType()->isArrayType()) {
-          // getBaseElementTypeUnsafe(...) handles multi-dimensonal arrays for us.
-          FromRecordDecl = FieldFrom->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl();
-          ToRecordDecl = FieldTo->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl();
-        }
-
-        if (!FromRecordDecl || !ToRecordDecl) {
-          const RecordType *RecordFrom =
-              FieldFrom->getType()->getAs<RecordType>();
-          const RecordType *RecordTo = FieldTo->getType()->getAs<RecordType>();
-
-          if (RecordFrom && RecordTo) {
-            FromRecordDecl = RecordFrom->getDecl();
-            ToRecordDecl = RecordTo->getDecl();
-          }
-        }
-
-        if (FromRecordDecl && ToRecordDecl) {
-          if (FromRecordDecl->isCompleteDefinition() &&
-              !ToRecordDecl->isCompleteDefinition()) {
-            Error Err = ImportDefinition(FromRecordDecl, ToRecordDecl);
-
-            if (Err && AccumulateChildErrors)
-              ChildErrors =  joinErrors(std::move(ChildErrors), std::move(Err));
-            else
-              consumeError(std::move(Err));
-          }
-        }
-      }
-    } else {
-      if (AccumulateChildErrors)
-        ChildErrors =
-            joinErrors(std::move(ChildErrors), ImportedOrErr.takeError());
-      else
-        consumeError(ImportedOrErr.takeError());
-    }
-  }
 
   // We reorder declarations in RecordDecls because they may have another order
   // in the "to" context than they have in the "from" context. This may happen
   // e.g when we import a class like this:
   //    struct declToImport {
   //        int a = c + b;
   //        int b = 1;
   //        int c = 2;