Author: martong Date: Wed Aug 22 04:52:14 2018 New Revision: 340402 URL: http://llvm.org/viewvc/llvm-project?rev=340402&view=rev Log: Fix import of class templates partial specialization
Summary: Currently there are several issues with the import of class template specializations. (1) Different TUs may have class template specializations with the same template arguments, but with different set of instantiated MethodDecls and FieldDecls. In this patch we provide a fix to merge these methods and fields. (2) Currently, we search the partial template specializations in the set of simple specializations and we add partial specializations as simple specializations. This is bad, this patch fixes it. Reviewers: a_sidorin, xazax.hun, r.stahl Subscribers: rnkovacs, dkrupp, cfe-commits Differential Revision: https://reviews.llvm.org/D50451 Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=340402&r1=340401&r2=340402&view=diff ============================================================================== --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original) +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Aug 22 04:52:14 2018 @@ -420,6 +420,25 @@ extern const internal::VariadicDynCastAl Decl, ClassTemplateSpecializationDecl> classTemplateSpecializationDecl; +/// Matches C++ class template partial specializations. +/// +/// Given +/// \code +/// template<class T1, class T2, int I> +/// class A {}; +/// +/// template<class T, int I> +/// class A<T, T*, I> {}; +/// +/// template<> +/// class A<int, int, 1> {}; +/// \endcode +/// classTemplatePartialSpecializationDecl() +/// matches the specialization \c A<T,T*,I> but not \c A<int,int,1> +extern const internal::VariadicDynCastAllOfMatcher< + Decl, ClassTemplatePartialSpecializationDecl> + classTemplatePartialSpecializationDecl; + /// Matches declarator declarations (field, variable, function /// and non-type template parameter declarations). /// Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=340402&r1=340401&r2=340402&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Aug 22 04:52:14 2018 @@ -2890,6 +2890,22 @@ Decl *ASTNodeImporter::VisitFieldDecl(Fi if (Importer.IsStructurallyEquivalent(D->getType(), FoundField->getType())) { Importer.MapImported(D, FoundField); + // In case of a FieldDecl of a ClassTemplateSpecializationDecl, the + // initializer of a FieldDecl might not had been instantiated in the + // "To" context. However, the "From" context might instantiated that, + // thus we have to merge that. + if (Expr *FromInitializer = D->getInClassInitializer()) { + // We don't have yet the initializer set. + if (FoundField->hasInClassInitializer() && + !FoundField->getInClassInitializer()) { + Expr *ToInitializer = Importer.Import(FromInitializer); + if (!ToInitializer) + // We can't return a nullptr here, + // since we already mapped D as imported. + return FoundField; + FoundField->setInClassInitializer(ToInitializer); + } + } return FoundField; } @@ -4544,27 +4560,50 @@ Decl *ASTNodeImporter::VisitClassTemplat // Try to find an existing specialization with these template arguments. void *InsertPos = nullptr; - ClassTemplateSpecializationDecl *D2 - = ClassTemplate->findSpecialization(TemplateArgs, InsertPos); - if (D2) { - // We already have a class template specialization with these template - // arguments. - - // FIXME: Check for specialization vs. instantiation errors. - - if (RecordDecl *FoundDef = D2->getDefinition()) { - if (!D->isCompleteDefinition() || IsStructuralMatch(D, FoundDef)) { - // The record types structurally match, or the "from" translation - // unit only had a forward declaration anyway; call it the same - // function. - return Importer.MapImported(D, FoundDef); - } - } - } else { - // Create a new specialization. - if (auto *PartialSpec = - dyn_cast<ClassTemplatePartialSpecializationDecl>(D)) { - // Import TemplateArgumentListInfo + ClassTemplateSpecializationDecl *D2 = nullptr; + ClassTemplatePartialSpecializationDecl *PartialSpec = + dyn_cast<ClassTemplatePartialSpecializationDecl>(D); + if (PartialSpec) + D2 = ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos); + else + D2 = ClassTemplate->findSpecialization(TemplateArgs, InsertPos); + ClassTemplateSpecializationDecl * const PrevDecl = D2; + RecordDecl *FoundDef = D2 ? D2->getDefinition() : nullptr; + if (FoundDef) { + if (!D->isCompleteDefinition()) { + // The "From" translation unit only had a forward declaration; call it + // the same declaration. + // TODO Handle the redecl chain properly! + return Importer.MapImported(D, FoundDef); + } + + if (IsStructuralMatch(D, FoundDef)) { + + Importer.MapImported(D, FoundDef); + + // Import those those default field initializers which have been + // instantiated in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) + Importer.Import(FromField); + + // Import those methods which have been instantiated in the + // "From" context, but not in the "To" context. + for (CXXMethodDecl *FromM : D->methods()) + Importer.Import(FromM); + + // TODO Import instantiated default arguments. + // TODO Import instantiated exception specifications. + // + // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint what + // else could be fused during an AST merge. + + return FoundDef; + } + } else { // We either couldn't find any previous specialization in the "To" + // context, or we found one but without definition. Let's create a + // new specialization and register that at the class template. + if (PartialSpec) { + // Import TemplateArgumentListInfo. TemplateArgumentListInfo ToTAInfo; const auto &ASTTemplateArgs = *PartialSpec->getTemplateArgsAsWritten(); if (ImportTemplateArgumentListInfo(ASTTemplateArgs, ToTAInfo)) @@ -4585,21 +4624,32 @@ Decl *ASTNodeImporter::VisitClassTemplat D2, D, Importer.getToContext(), D->getTagKind(), DC, StartLoc, IdLoc, ToTPList, ClassTemplate, llvm::makeArrayRef(TemplateArgs.data(), TemplateArgs.size()), - ToTAInfo, CanonInjType, nullptr)) + ToTAInfo, CanonInjType, + cast_or_null<ClassTemplatePartialSpecializationDecl>(PrevDecl))) return D2; - } else { + // Update InsertPos, because preceding import calls may have invalidated + // it by adding new specializations. + if (!ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos)) + // Add this partial specialization to the class template. + ClassTemplate->AddPartialSpecialization( + cast<ClassTemplatePartialSpecializationDecl>(D2), InsertPos); + + } else { // Not a partial specialization. if (GetImportedOrCreateDecl( D2, D, Importer.getToContext(), D->getTagKind(), DC, StartLoc, - IdLoc, ClassTemplate, TemplateArgs, /*PrevDecl=*/nullptr)) + IdLoc, ClassTemplate, TemplateArgs, PrevDecl)) return D2; + + // Update InsertPos, because preceding import calls may have invalidated + // it by adding new specializations. + if (!ClassTemplate->findSpecialization(TemplateArgs, InsertPos)) + // Add this specialization to the class template. + ClassTemplate->AddSpecialization(D2, InsertPos); } D2->setSpecializationKind(D->getSpecializationKind()); - // Add this specialization to the class template. - ClassTemplate->AddSpecialization(D2, InsertPos); - // Import the qualifier, if any. D2->setQualifierInfo(Importer.Import(D->getQualifierLoc())); Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=340402&r1=340401&r2=340402&view=diff ============================================================================== --- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original) +++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Wed Aug 22 04:52:14 2018 @@ -573,6 +573,9 @@ const internal::VariadicDynCastAllOfMatc const internal::VariadicDynCastAllOfMatcher<Decl, ClassTemplateSpecializationDecl> classTemplateSpecializationDecl; +const internal::VariadicDynCastAllOfMatcher< + Decl, ClassTemplatePartialSpecializationDecl> + classTemplatePartialSpecializationDecl; const internal::VariadicDynCastAllOfMatcher<Decl, DeclaratorDecl> declaratorDecl; const internal::VariadicDynCastAllOfMatcher<Decl, ParmVarDecl> parmVarDecl; Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=340402&r1=340401&r2=340402&view=diff ============================================================================== --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Wed Aug 22 04:52:14 2018 @@ -2955,12 +2955,12 @@ TEST_P( auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match( FromTu, classTemplateDecl(hasName("declToImport"))); auto *ToD = Import(FromD, Lang_CXX); - + auto Pattern = classTemplateDecl( has(cxxRecordDecl(has(friendDecl(has(classTemplateDecl())))))); ASSERT_TRUE(MatchVerifier<Decl>{}.match(FromD, Pattern)); EXPECT_TRUE(MatchVerifier<Decl>{}.match(ToD, Pattern)); - + auto *Class = FirstDeclMatcher<ClassTemplateDecl>().match(ToD, classTemplateDecl()); auto *Friend = FirstDeclMatcher<FriendDecl>().match(ToD, friendDecl()); @@ -2968,6 +2968,263 @@ TEST_P( EXPECT_EQ(Friend->getFriendDecl()->getPreviousDecl(), Class); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template <typename T> + struct X { + int a{0}; // FieldDecl with InitListExpr + X(char) : a(3) {} // (1) + X(int) {} // (2) + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr + X<char> xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl without InitlistExpr: + auto *ToField = *ToSpec->field_begin(); + ASSERT_TRUE(ToField); + ASSERT_FALSE(ToField->getInClassInitializer()); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr + X<char> xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl with InitlistExpr: + auto *FromField = *FromSpec->field_begin(); + ASSERT_TRUE(FromField); + ASSERT_TRUE(FromField->getInClassInitializer()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + EXPECT_EQ(ImportedSpec, ToSpec); + // After the import, the FieldDecl has to be merged, thus it should have the + // InitListExpr. + EXPECT_TRUE(ToField->getInClassInitializer()); +} + +TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template <typename T> + struct X { + void f() {} + void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X<char> x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X<char> x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher<FunctionDecl>().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher<FunctionDecl>().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template <typename T> + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X<char> { + int a; + }; + void foo() { + X<char> x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X<char> { + int b; + }; + void foo() { + X<char> x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + + // We expect one (ODR) warning during the import. + EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings()); + + // The second specialization is different from the first, thus it violates + // ODR, consequently we expect to keep the first specialization only, which is + // already in the "To" context. + EXPECT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template <typename T> + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X<char> x('c'); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X<char> x(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + // Match the void(int) ctor. + auto CtorPattern = + cxxConstructorDecl(hasParameter(0, varDecl(hasType(asString("int")))), + hasParent(classTemplateSpecializationDecl())); + auto *FromCtor = + FirstDeclMatcher<CXXConstructorDecl>().match(FromTU, CtorPattern); + auto *ToCtor = + FirstDeclMatcher<CXXConstructorDecl>().match(ToTU, CtorPattern); + ASSERT_TRUE(FromCtor->hasBody()); + ASSERT_FALSE(ToCtor->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToCtor->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ClassTemplatePartialSpecializationsShouldNotBeDuplicated) { + auto Code = + R"( + // primary template + template<class T1, class T2, int I> + class A {}; + + // partial specialization + template<class T, int I> + class A<T, T*, I> {}; + )"; + Decl *ToTU = getToTuDecl(Code, Lang_CXX11); + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromSpec = + FirstDeclMatcher<ClassTemplatePartialSpecializationDecl>().match( + FromTU, classTemplatePartialSpecializationDecl()); + auto *ToSpec = + FirstDeclMatcher<ClassTemplatePartialSpecializationDecl>().match( + ToTU, classTemplatePartialSpecializationDecl()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter<ClassTemplatePartialSpecializationDecl>().match( + ToTU, classTemplatePartialSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, ClassTemplateSpecializationsShouldNotBeDuplicated) { + auto Code = + R"( + // primary template + template<class T1, class T2, int I> + class A {}; + + // full specialization + template<> + class A<int, int, 1> {}; + )"; + Decl *ToTU = getToTuDecl(Code, Lang_CXX11); + Decl *FromTU = getTuDecl(Code, Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl()); + auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, ClassTemplateFullAndPartialSpecsShouldNotBeMixed) { + std::string PrimaryTemplate = + R"( + template<class T1, class T2, int I> + class A {}; + )"; + auto PartialSpec = + R"( + template<class T, int I> + class A<T, T*, I> {}; + )"; + auto FullSpec = + R"( + template<> + class A<int, int, 1> {}; + )"; + Decl *ToTU = getToTuDecl(PrimaryTemplate + FullSpec, Lang_CXX11); + Decl *FromTU = getTuDecl(PrimaryTemplate + PartialSpec, Lang_CXX11); + auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match( + FromTU, classTemplateSpecializationDecl()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + EXPECT_TRUE(ImportedSpec); + // Check the number of partial specializations. + EXPECT_EQ(1u, DeclCounter<ClassTemplatePartialSpecializationDecl>().match( + ToTU, classTemplatePartialSpecializationDecl())); + // Check the number of full specializations. + EXPECT_EQ(1u, DeclCounter<ClassTemplateSpecializationDecl>().match( + ToTU, classTemplateSpecializationDecl( + unless(classTemplatePartialSpecializationDecl())))); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits