[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

Wed Mar 9 01:29:38 PST 2022

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

So, my understanding is that the issue stems from the fact that `hasInClassInitializer()` gave inconsistent results with `getInClassInitializer()` for previously imported nodes. Please confirm. For that, the solution you provide seems reasonable and the test cases gives a proper coverage.

================
Comment at: clang/lib/AST/ASTImporter.cpp:3649-3650
         // thus we have to merge that.
+        // Note: `hasInClassInitializer()` is not the same as non-null
+        // `getInClassInitializer()` value.
         if (Expr *FromInitializer = D->getInClassInitializer()) {
----------------
It is unfortunate to have such an API. Anyway, the assertions you placed seems to address this mess. Good.

================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:536-547
+TEST_P(ImportExpr, ImportCXXDefaultInitExpr) {
+  MatchVerifier<Decl> Verifier;
+  testImport("class declToImport { int DefInit = 5; }; declToImport X;",
+             Lang_CXX11, "", Lang_CXX11, Verifier,
+             cxxRecordDecl(hasDescendant(cxxConstructorDecl(
+                 hasAnyConstructorInitializer(cxxCtorInitializer(
+                     withInitializer(cxxDefaultInitExpr())))))));
----------------
So these tests failed with the baseline?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120824/new/

https://reviews.llvm.org/D120824