[clang][AST] Add support for DecompositionDecl to ASTImporter.

[balazske]

Summary

BindingDecl was added recently but the related DecompositionDecl is needed
to make C++17 structured bindings importable.
Import of BindingDecl was changed to avoid infinite import loop.

[martong]

clang/lib/AST/ASTImporter.cpp

2305–2309

So, we moved the import of the binding before importing the decomposition decl to avoid an infinite recursion. But why can't we have an infinit recursion this way?

Perhaps, it would be useful to have a test case that triggered the infinity in case of the original order of the import calls.

[shafik]

clang/lib/AST/ASTImporter.cpp

2305–2309

Yes, I agree, I would also like to understand better why this avoids the infinite recursion problem, a test case would be helpful as well as an explanation of the steps that leads us to the problem.

[balazske]

clang/lib/AST/ASTImporter.cpp

2305–2309

With the import at original place, in VisitVarDecl the bindings (which are BindingDecl) are imported before create of the DecompositionDecl instance, and in VisitBindingDecl the decomposition (a DecompositionDecl that is VarDecl) is imported before create of the BindingDecl instance. This causes the recursion with the most simple import of a DecompositionDecl. This is triggered by the existing simple test (actually I discovered the problem at the test failure). If the decomposition is imported after create of the BindingDecl the same BindingDecl is not imported again because it already exists.

[martong]

clang/lib/AST/ASTImporter.cpp

2305–2309

Ok, thanks.

clang/lib/AST/ASTImporter.cpp

ExpectedDecl ASTNodeImporter::VisitBindingDecl(BindingDecl *D) {
   DeclarationName Name;
   SourceLocation Loc;
   NamedDecl *ToND;
   if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToND, Loc))
     return std::move(Err);
   if (ToND)
     return ToND;
 
+  BindingDecl *ToD;
+  if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
+                              Name.getAsIdentifierInfo()))
+    return ToD;
+
   Error Err = Error::success();
   QualType ToType = importChecked(Err, D->getType());
   Expr *ToBinding = importChecked(Err, D->getBinding());
   ValueDecl *ToDecomposedDecl = importChecked(Err, D->getDecomposedDecl());
   if (Err)
     return std::move(Err);
 
-  BindingDecl *ToD;
-  if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
-                              Name.getAsIdentifierInfo()))
-    return ToD;
-
   ToD->setBinding(ToType, ToBinding);
   ToD->setDecomposedDecl(ToDecomposedDecl);
   addDeclToContexts(D, ToD);
 
   return ToD;
 }
 
 ExpectedDecl ASTNodeImporter::VisitAccessSpecDecl(AccessSpecDecl *D) {
ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) {
   Error Err = Error::success();
   auto ToType = importChecked(Err, D->getType());
   auto ToTypeSourceInfo = importChecked(Err, D->getTypeSourceInfo());
   auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart());
   auto ToQualifierLoc = importChecked(Err, D->getQualifierLoc());
   if (Err)
     return std::move(Err);
 
-  // Create the imported variable.
   VarDecl *ToVar;
+  if (auto *FromDecomp = dyn_cast<DecompositionDecl>(D)) {
+    SmallVector<BindingDecl *> Bindings(FromDecomp->bindings().size());
+    if (Error Err =
+            ImportArrayChecked(FromDecomp->bindings(), Bindings.begin()))
+      return std::move(Err);
+    DecompositionDecl *ToDecomp;
+    if (GetImportedOrCreateDecl(
+            ToDecomp, FromDecomp, Importer.getToContext(), DC, ToInnerLocStart,
+            Loc, ToType, ToTypeSourceInfo, D->getStorageClass(), Bindings))
+      return ToDecomp;
+    ToVar = ToDecomp;
+  } else {
+    // Create the imported variable.
     if (GetImportedOrCreateDecl(ToVar, D, Importer.getToContext(), DC,
                                 ToInnerLocStart, Loc,
-                              Name.getAsIdentifierInfo(),
-                              ToType, ToTypeSourceInfo,
-                              D->getStorageClass()))
+                                Name.getAsIdentifierInfo(), ToType,
+                                ToTypeSourceInfo, D->getStorageClass()))
       return ToVar;
+  }
 
   ToVar->setTSCSpec(D->getTSCSpec());
   ToVar->setQualifierInfo(ToQualifierLoc);
   ToVar->setAccess(D->getAccess());
   ToVar->setLexicalDeclContext(LexicalDC);
 
   if (FoundByLookup) {
     auto *Recent = const_cast<VarDecl *>(FoundByLookup->getMostRecentDecl());

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ImportVariables, ImportBindingDecl) {
   VerifyImport("x1");
   VerifyImport("y1");
   VerifyImport("x2");
   VerifyImport("y2");
   VerifyImport("x3");
   VerifyImport("y3");
 }
 
+TEST_P(ImportVariables, ImportDecompositionDeclArray) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      void declToImport() {
+        int a[2] = {1,2};
+        auto [x1,y1] = a;
+      };
+      )",
+      Lang_CXX17, "", Lang_CXX17);
+
+  TranslationUnitDecl *FromTU = From->getTranslationUnitDecl();
+  auto *FromDecomp =
+      FirstDeclMatcher<DecompositionDecl>().match(FromTU, decompositionDecl());
+  auto *ToDecomp = Import(FromDecomp, Lang_CXX17);
+  EXPECT_TRUE(ToDecomp);
+
+  ArrayRef<BindingDecl *> FromB = FromDecomp->bindings();
+  ArrayRef<BindingDecl *> ToB = ToDecomp->bindings();
+  EXPECT_EQ(FromB.size(), ToB.size());
+  for (unsigned int I = 0; I < FromB.size(); ++I) {
+    auto *ToBI = Import(FromB[I], Lang_CXX17);
+    EXPECT_EQ(ToBI, ToB[I]);
+  }
+}
+
 struct ImportClasses : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ImportClasses, ImportDefinitionWhenProtoIsInNestedToContext) {
   Decl *ToTU = getToTuDecl("struct A { struct X *Xp; };", Lang_C99);
   Decl *FromTU1 = getTuDecl("struct X {};", Lang_C99, "input1.cc");
   auto Pattern = recordDecl(hasName("X"), unless(isImplicit()));
   auto ToProto = FirstDeclMatcher<RecordDecl>().match(ToTU, Pattern);
   auto FromDef = FirstDeclMatcher<RecordDecl>().match(FromTU1, Pattern);