balazske updated this revision to Diff 492106.
balazske added a comment.
New patch after more thorough debugging.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140562/new/
https://reviews.llvm.org/D140562
Files:
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8084,6 +8084,247 @@
EXPECT_FALSE(SharedStatePtr->isNewDecl(ToBar));
}
+struct ImportInjectedClassNameType : public ASTImporterOptionSpecificTestBase {
+protected:
+ const CXXRecordDecl *findInjected(const CXXRecordDecl *Parent) {
+ for (Decl *Found : Parent->decls()) {
+ const auto *Record = dyn_cast<CXXRecordDecl>(Found);
+ if (Record && Record->isInjectedClassName())
+ return Record;
+ }
+ return nullptr;
+ }
+
+ void checkInjType(const CXXRecordDecl *D) {
+ // The whole redecl chain should have the same InjectedClassNameType
+ // instance. The injected record declaration is a separate chain, this
+ // should contain the same type too.
+ const Type *Ty = nullptr;
+ for (const Decl *ReD : D->redecls()) {
+ const auto *ReRD = cast<CXXRecordDecl>(ReD);
+ EXPECT_TRUE(ReRD->getTypeForDecl());
+ EXPECT_TRUE(!Ty || Ty == ReRD->getTypeForDecl());
+ Ty = ReRD->getTypeForDecl();
+ }
+ ASSERT_TRUE(Ty);
+ const auto *InjTy = Ty->castAs<InjectedClassNameType>();
+ EXPECT_TRUE(InjTy);
+ if (CXXRecordDecl *Def = D->getDefinition()) {
+ const CXXRecordDecl *InjRD = findInjected(Def);
+ EXPECT_TRUE(InjRD);
+ EXPECT_EQ(InjRD->getTypeForDecl(), InjTy);
+ }
+ }
+
+ void testImport(Decl *ToTU, Decl *FromTU, Decl *FromD) {
+ checkInjType(cast<CXXRecordDecl>(FromD));
+ Decl *ToD = Import(FromD, Lang_CXX11);
+ if (auto *ToRD = dyn_cast<CXXRecordDecl>(ToD))
+ checkInjType(ToRD);
+ }
+
+ const char *ToCodeA =
+ R"(
+ template <class T>
+ struct A;
+ )";
+ const char *ToCodeADef =
+ R"(
+ template <class T>
+ struct A {
+ typedef A T1;
+ };
+ )";
+ const char *ToCodeC =
+ R"(
+ template <class T>
+ struct C;
+ )";
+ const char *ToCodeCDef =
+ R"(
+ template <class T>
+ struct A {
+ typedef A T1;
+ };
+
+ template <class T1, class T2>
+ struct B {};
+
+ template<class T>
+ struct C {
+ typedef typename A<T>::T1 T1;
+ typedef B<T1, T> T2;
+ typedef B<T1, C> T3;
+ };
+ )";
+ const char *FromCode =
+ R"(
+ template <class T>
+ struct A;
+ template <class T>
+ struct A {
+ typedef A T1;
+ };
+ template <class T>
+ struct A;
+
+ template <class T1, class T2>
+ struct B {};
+
+ template <class T>
+ struct C;
+ template <class T>
+ struct C {
+ typedef typename A<T>::T1 T1;
+ typedef B<T1, T> T2;
+ typedef B<T1, C> T3;
+ };
+ template <class T>
+ struct C;
+
+ template <class T>
+ struct D {
+ void f(typename C<T>::T3 *);
+ };
+ )";
+};
+
+TEST_P(ImportInjectedClassNameType, ImportADef) {
+ Decl *ToTU = getToTuDecl(ToCodeA, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A"), isDefinition()));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportAFirst) {
+ Decl *ToTU = getToTuDecl(ToCodeA, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportALast) {
+ Decl *ToTU = getToTuDecl(ToCodeA, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = LastDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportADefToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeADef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A"), isDefinition()));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportAFirstToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeADef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportALastToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeADef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromA = LastDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+ testImport(ToTU, FromTU, FromA);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportCDef) {
+ Decl *ToTU = getToTuDecl(ToCodeC, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromC = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("C"), isDefinition()));
+ testImport(ToTU, FromTU, FromC);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportCLast) {
+ Decl *ToTU = getToTuDecl(ToCodeC, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromC = LastDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("C")));
+ testImport(ToTU, FromTU, FromC);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportCDefToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeCDef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromC = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("C"), isDefinition()));
+ testImport(ToTU, FromTU, FromC);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportCLastToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeCDef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromC = LastDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("C")));
+ testImport(ToTU, FromTU, FromC);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportD) {
+ Decl *ToTU = getToTuDecl("", Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromD = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("D"), isDefinition()));
+ testImport(ToTU, FromTU, FromD);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportDToDef) {
+ Decl *ToTU = getToTuDecl(ToCodeCDef, Lang_CXX11);
+ Decl *FromTU = getTuDecl(FromCode, Lang_CXX11);
+ auto *FromD = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("D"), isDefinition()));
+ testImport(ToTU, FromTU, FromD);
+}
+
+TEST_P(ImportInjectedClassNameType, ImportTypedefType) {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ template <class T>
+ struct A {
+ typedef A A1;
+ void f(A1 *);
+ };
+ )",
+ Lang_CXX11);
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <class T>
+ struct A {
+ typedef A A1;
+ void f(A1 *);
+ };
+ template<class T>
+ void A<T>::f(A::A1 *) {}
+ )",
+ Lang_CXX11);
+
+ auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f"), isDefinition()));
+ auto *ToF = Import(FromF, Lang_CXX11);
+ EXPECT_TRUE(ToF);
+ ASTContext &ToCtx = ToF->getDeclContext()->getParentASTContext();
+
+ auto *ToA1 =
+ FirstDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("A1")));
+ QualType ToInjTypedef = ToA1->getUnderlyingType().getCanonicalType();
+ QualType ToInjParmVar =
+ ToF->parameters()[0]->getType().getDesugaredType(ToCtx);
+ ToInjParmVar =
+ ToInjParmVar->getAs<PointerType>()->getPointeeType().getCanonicalType();
+ EXPECT_TRUE(isa<InjectedClassNameType>(ToInjTypedef));
+ EXPECT_TRUE(isa<InjectedClassNameType>(ToInjParmVar));
+ EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
@@ -8161,5 +8402,8 @@
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportAttributes,
DefaultTestValuesForRunOptions);
+INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportInjectedClassNameType,
+ DefaultTestValuesForRunOptions);
+
} // end namespace ast_matchers
} // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1460,20 +1460,11 @@
if (!ToDeclOrErr)
return ToDeclOrErr.takeError();
- ExpectedType ToInjTypeOrErr = import(T->getInjectedSpecializationType());
- if (!ToInjTypeOrErr)
- return ToInjTypeOrErr.takeError();
-
- // FIXME: ASTContext::getInjectedClassNameType is not suitable for AST reading
- // See comments in InjectedClassNameType definition for details
- // return Importer.getToContext().getInjectedClassNameType(D, InjType);
- enum {
- TypeAlignmentInBits = 4,
- TypeAlignment = 1 << TypeAlignmentInBits
- };
-
- return QualType(new (Importer.getToContext(), TypeAlignment)
- InjectedClassNameType(*ToDeclOrErr, *ToInjTypeOrErr), 0);
+ // The InjectedClassNameType is created in VisitRecordDecl when the
+ // T->getDecl() is imported. Here we can return the existing type.
+ const Type *Ty = (*ToDeclOrErr)->getTypeForDecl();
+ assert(Ty && isa<InjectedClassNameType>(Ty));
+ return QualType(Ty, 0);
}
ExpectedType ASTNodeImporter::VisitRecordType(const RecordType *T) {
@@ -2948,8 +2939,6 @@
// InjectedClassNameType (see Sema::CheckClassTemplate). Update the
// previously set type to the correct value here (ToDescribed is not
// available at record create).
- // FIXME: The previous type is cleared but not removed from
- // ASTContext's internal storage.
CXXRecordDecl *Injected = nullptr;
for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
auto *Record = dyn_cast<CXXRecordDecl>(Found);
@@ -2959,20 +2948,34 @@
}
}
// Create an injected type for the whole redecl chain.
+ // The chain may contain an already existing injected type at the start,
+ // if yes this should be reused. We must ensure that only one type
+ // object exists for the injected type (including the injected record
+ // declaration), ASTContext does not check it.
SmallVector<Decl *, 2> Redecls =
getCanonicalForwardRedeclChain(D2CXX);
+ const Type *FrontTy =
+ cast<CXXRecordDecl>(Redecls.front())->getTypeForDecl();
+ QualType InjSpec;
+ if (auto *InjTy = FrontTy->getAs<InjectedClassNameType>())
+ InjSpec = InjTy->getInjectedSpecializationType();
+ else
+ InjSpec = ToDescribed->getInjectedClassNameSpecialization();
for (auto *R : Redecls) {
auto *RI = cast<CXXRecordDecl>(R);
- RI->setTypeForDecl(nullptr);
- // Below we create a new injected type and assign that to the
- // canonical decl, subsequent declarations in the chain will reuse
- // that type.
- Importer.getToContext().getInjectedClassNameType(
- RI, ToDescribed->getInjectedClassNameSpecialization());
+ if (R != Redecls.front() ||
+ !isa<InjectedClassNameType>(RI->getTypeForDecl()))
+ RI->setTypeForDecl(nullptr);
+ // This function tries to get the injected type from getTypeForDecl,
+ // then from the previous declaration if possible. If not, it creates
+ // a new type.
+ Importer.getToContext().getInjectedClassNameType(RI, InjSpec);
}
- // Set the new type for the previous injected decl too.
+ // Set the new type for the injected decl too.
if (Injected) {
Injected->setTypeForDecl(nullptr);
+ // This function will copy the injected type from D2CXX into Injected.
+ // The injected decl does not have a previous decl to copy from.
Importer.getToContext().getTypeDeclType(Injected, D2CXX);
}
}
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4624,8 +4624,7 @@
Decl->TypeForDecl = PrevDecl->TypeForDecl;
assert(isa<InjectedClassNameType>(Decl->TypeForDecl));
} else {
- Type *newType =
- new (*this, TypeAlignment) InjectedClassNameType(Decl, TST);
+ Type *newType = new (*this, TypeAlignment) InjectedClassNameType(Decl, TST);
Decl->TypeForDecl = newType;
Types.push_back(newType);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
