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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
