[ASTImporter] Set redecl chain of functions before any other import

[martong]

Summary

FunctionDecl import starts with a lookup and then we create a new Decl.
Then in case of CXXConstructorDecl we further import other Decls
(base classes, members through CXXConstructorDecl::inits()) before connecting
the redecl chain. During those in-between imports structural eq fails
because the canonical decl is different. This commit fixes this.
Synthesizing a test seemed extremely hard, however, Xerces analysis
reproduces the problem.

[a_sidorin]

Hi Gabor,
The change looks fine. Thanks!

[shafik]

Did you ever resolve the issue of libcxx tests not running https://reviews.llvm.org/D53697

[martong]

Did you ever resolve the issue of libcxx tests not running https://reviews.llvm.org/D53697

Hi @shafik ,
Sorry for the late reaction, I was on a two weeks long vacation recently.
My priority is to make those libcxx tests pass, I did not forget. :)

[martong]

Did you ever resolve the issue of libcxx tests not running https://reviews.llvm.org/D53697

Oh, by the way this change is completely independent from that other patch.

lib/AST/ASTImporter.cpp

ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   FunctionDecl *ToFunction = nullptr;
   if (auto *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     if (GetImportedOrCreateDecl<CXXConstructorDecl>(
         ToFunction, D, Importer.getToContext(), cast<CXXRecordDecl>(DC),
         ToInnerLocStart, NameInfo, T, TInfo,
         FromConstructor->isExplicit(),
         D->isInlineSpecified(), D->isImplicit(), D->isConstexpr()))
       return ToFunction;
-    if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
-      SmallVector<CXXCtorInitializer *, 4> CtorInitializers(NumInitializers);
-      // Import first, then allocate memory and copy if there was no error.
-      if (Error Err = ImportContainerChecked(
-          FromConstructor->inits(), CtorInitializers))
-        return std::move(Err);
-      auto **Memory =
-          new (Importer.getToContext()) CXXCtorInitializer *[NumInitializers];
-      std::copy(CtorInitializers.begin(), CtorInitializers.end(), Memory);
-      auto *ToCtor = cast<CXXConstructorDecl>(ToFunction);
-      ToCtor->setCtorInitializers(Memory);
-      ToCtor->setNumCtorInitializers(NumInitializers);
-    }
   } else if (isa<CXXDestructorDecl>(D)) {
     if (GetImportedOrCreateDecl<CXXDestructorDecl>(
         ToFunction, D, Importer.getToContext(), cast<CXXRecordDecl>(DC),
         ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(),
         D->isImplicit()))
       return ToFunction;
   } else if (CXXConversionDecl *FromConversion =
                  dyn_cast<CXXConversionDecl>(D)) {
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   } else {
     if (GetImportedOrCreateDecl(ToFunction, D, Importer.getToContext(), DC,
                                 ToInnerLocStart, NameInfo, T, TInfo,
                                 D->getStorageClass(), D->isInlineSpecified(),
                                 D->hasWrittenPrototype(), D->isConstexpr()))
       return ToFunction;
   }
 
+  // Connect the redecl chain.
+  if (FoundByLookup) {
+    auto *Recent = const_cast<FunctionDecl *>(
+          FoundByLookup->getMostRecentDecl());
+    ToFunction->setPreviousDecl(Recent);
+  }
+
+  // Import Ctor initializers.
+  if (auto *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
+    if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
+      SmallVector<CXXCtorInitializer *, 4> CtorInitializers(NumInitializers);
+      // Import first, then allocate memory and copy if there was no error.
+      if (Error Err = ImportContainerChecked(
+          FromConstructor->inits(), CtorInitializers))
+        return std::move(Err);
+      auto **Memory =
+          new (Importer.getToContext()) CXXCtorInitializer *[NumInitializers];
+      std::copy(CtorInitializers.begin(), CtorInitializers.end(), Memory);
+      auto *ToCtor = cast<CXXConstructorDecl>(ToFunction);
+      ToCtor->setCtorInitializers(Memory);
+      ToCtor->setNumCtorInitializers(NumInitializers);
+    }
+  }
+
   ToFunction->setQualifierInfo(ToQualifierLoc);
   ToFunction->setAccess(D->getAccess());
   ToFunction->setLexicalDeclContext(LexicalDC);
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
   ToFunction->setRangeEnd(ToEndLoc);
 
   // Set the parameters.
   for (auto *Param : Parameters) {
     Param->setOwningFunction(ToFunction);
     ToFunction->addDeclInternal(Param);
   }
   ToFunction->setParams(Parameters);
 
-  if (FoundByLookup) {
-    auto *Recent = const_cast<FunctionDecl *>(
-          FoundByLookup->getMostRecentDecl());
-    ToFunction->setPreviousDecl(Recent);
-  }
-
   // We need to complete creation of FunctionProtoTypeLoc manually with setting
   // params it refers to.
   if (TInfo) {
     if (auto ProtoLoc =
         TInfo->getTypeLoc().IgnoreParens().getAs<FunctionProtoTypeLoc>()) {
       for (unsigned I = 0, N = Parameters.size(); I != N; ++I)
         ProtoLoc.setParam(I, Parameters[I]);
     }