[clang][ASTImporter] Fix import of recursive field initializer.

[balazske]

Summary

Import of field initializers with circular reference was not working,
this is fixed now.

Fixes issue #63120

[steakhal]

LGTM

[balazske]

using clang-format

[danix800]

clang/lib/AST/ASTImporter.cpp

3936–3937

Initializer could indirectly depends on this field and set the initializer while importing.
setInClassInitializer() asserts that initializer should not be set more than once:

static int ref_A();
static int ref_B();
struct A {
  int a = ref_B();
};
struct B {
  int b = ref_A();
};
int ref_B() { B b; return b.b; }
int ref_A() { A a; return a.a; }

[balazske]

clang/lib/AST/ASTImporter.cpp

3936–3937

This example code really causes problems. But import of Expr is not checked for recursion, the assertion in the new code fails for this test.

Why do you want to use such code? It looks to cause infinite loop when executed. Even code like class A { int b{b}; }; is probably not correct.

[danix800]

clang/lib/AST/ASTImporter.cpp

3936–3937

Like int a{a}; this testcase is minimized to just show what might cause the problem.

[balazske]

Added check for duplicate import of initializer.
Added more tests.

clang/lib/AST/ASTImporter.cpp

 //===- ASTImporter.cpp - Importing ASTs from other Contexts ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
for (auto *FoundDecl : FoundDecls) {
     }
   }
 
   Error Err = Error::success();
   auto ToType = importChecked(Err, D->getType());
   auto ToTInfo = importChecked(Err, D->getTypeSourceInfo());
   auto ToBitWidth = importChecked(Err, D->getBitWidth());
   auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart());
-  auto ToInitializer = importChecked(Err, D->getInClassInitializer());
   if (Err)
     return std::move(Err);
   const Type *ToCapturedVLAType = nullptr;
   if (Error Err = Importer.importInto(
           ToCapturedVLAType, cast_or_null<Type>(D->getCapturedVLAType())))
     return std::move(Err);
 
   FieldDecl *ToField;
   if (GetImportedOrCreateDecl(ToField, D, Importer.getToContext(), DC,
                               ToInnerLocStart, Loc, Name.getAsIdentifierInfo(),
                               ToType, ToTInfo, ToBitWidth, D->isMutable(),
                               D->getInClassInitStyle()))
     return ToField;
 
   // We need [[no_unqiue_address]] attributes to be added to FieldDecl, before
   // we add fields in CXXRecordDecl::addedMember, otherwise record will be
   // marked as having non-zero size.
   Err = Importer.ImportAttrs(ToField, D);
   if (Err)
     return std::move(Err);
   ToField->setAccess(D->getAccess());
   ToField->setLexicalDeclContext(LexicalDC);
-  if (ToInitializer)
-    ToField->setInClassInitializer(ToInitializer);
   ToField->setImplicit(D->isImplicit());
   if (ToCapturedVLAType)
     ToField->setCapturedVLAType(cast<VariableArrayType>(ToCapturedVLAType));
   LexicalDC->addDeclInternal(ToField);
+  // Import initializer only after the field was created, it may have recursive
+  // reference to the field.
+  auto ToInitializer = importChecked(Err, D->getInClassInitializer());
+  if (Err)
+    return std::move(Err);
+  if (ToInitializer) {
+    auto *AlreadyImported = ToField->getInClassInitializer();
+    if (AlreadyImported)
+      assert(ToInitializer == AlreadyImported &&
+             "Duplicate import of in-class initializer.");
+    else
+      ToField->setInClassInitializer(ToInitializer);
+  }
+
   return ToField;
 }
 
 ExpectedDecl ASTNodeImporter::VisitIndirectFieldDecl(IndirectFieldDecl *D) {
   // Import the major distinguishing characteristics of a variable.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;

clang/unittests/AST/ASTImporterTest.cpp

 //===- unittest/AST/ASTImporterTest.cpp - AST node import test ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
Decl *FromTU = getTuDecl(
       Lang_CXX14, "input1.cc");
   auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
       FromTU, functionDecl(hasName("f"), isDefinition()));
   auto *ToF = Import(FromF, Lang_CXX11);
   EXPECT_TRUE(ToF);
   EXPECT_TRUE(ToX->getInClassInitializer());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportRecursiveFieldInitializer) {
+  const char *Code =
+      R"(
+      struct AP_TECS;
+
+      struct AP_Landing {
+        AP_TECS *TECS_controller;
+      };
+
+      struct AP_TECS {
+        AP_Landing landing;
+      };
+
+      class Plane {
+        AP_TECS TECS_controller{landing};
+        AP_Landing landing{&TECS_controller};
+      };
+      )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromR = FirstDeclMatcher<CXXRecordDecl>().match(
+      FromTU, cxxRecordDecl(hasName("Plane")));
+  for (FieldDecl *F : FromR->fields())
+    EXPECT_TRUE(F->getInClassInitializer());
+  auto *ToR = Import(FromR, Lang_CXX11);
+  for (FieldDecl *F : ToR->fields())
+    EXPECT_TRUE(F->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportFieldInitializerWithItself) {
+  const char *Code =
+      R"(
+      class A {
+        int a{a};
+      };
+      )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+  auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+      FromTU, cxxRecordDecl(hasName("A")));
+  EXPECT_TRUE(FromA->field_begin()->getInClassInitializer());
+  auto *ToA = Import(FromA, Lang_CXX11);
+  EXPECT_TRUE(ToA->field_begin()->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportRecursiveFieldInitializer1) {
+  // FIXME: This is a example of recursive field initialization that is not
+  // supported.
+  // The following import chain occurs (not complete):
+  // import of A => A.a => in-class initializer of A.a => ref_B() => B => B.b
+  // => in-class initializer of B.b => ref_A() => CXXConstructExpr for A =>
+  // CXXDefaultInitExpr for A.a => in-class initializer of A.a
+  // in-class initializer of A.a is created in two different instances in this
+  // case (import of FieldDecl and CXXDefaultInitExpr). Probably not a big
+  // problem because it is an Expr (the second construction can be ignored
+  // instead of assert). But such recursive init code should not occur in
+  // practice.
+  const char *Code =
+      R"(
+      static int ref_A();
+      static int ref_B();
+      struct A {
+        int a = ref_B();
+      };
+      struct B {
+        int b = ref_A();
+      };
+      int ref_B() { B b; return b.b; }
+      int ref_A() { A a; return a.a; }
+      )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+  auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+      FromTU, cxxRecordDecl(hasName("A")));
+  EXPECT_TRUE(FromA->field_begin()->getInClassInitializer());
+  // auto *ToA = Import(FromA, Lang_CXX11);
+  // EXPECT_TRUE(ToA->field_begin()->getInClassInitializer());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, isNewDecl) {
   Decl *FromTU = getTuDecl(
       R"(
       int bar() {
         return 0;
       }
       void other() {
         bar();