[PATCH] D49792: [ASTmporter] SourceRange-free function parameter checking for declarations
This revision was automatically updated to reflect the committed changes. Closed by commit rL339018: [ASTmporter] SourceRange-free function parameter checking for declarations (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49792?vs=157236&id=159303#toc Repository: rL LLVM https://reviews.llvm.org/D49792 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -1147,15 +1147,21 @@ FunctionDecl *FunDecl; if (isa(D) && (FunDecl = dyn_cast(OrigDC)) && FunDecl->hasBody()) { -SourceRange RecR = D->getSourceRange(); -SourceRange BodyR = FunDecl->getBody()->getSourceRange(); -// If RecordDecl is not in Body (it is a param), we bail out. -if (RecR.isValid() && BodyR.isValid() && -(RecR.getBegin() < BodyR.getBegin() || - BodyR.getEnd() < RecR.getEnd())) { - Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node) - << D->getDeclKindName(); - return true; +auto getLeafPointeeType = [](const Type *T) { + while (T->isPointerType() || T->isArrayType()) { +T = T->getPointeeOrArrayElementType(); + } + return T; +}; +for (const ParmVarDecl *P : FunDecl->parameters()) { + const Type *LeafT = + getLeafPointeeType(P->getType().getCanonicalType().getTypePtr()); + auto *RT = dyn_cast(LeafT); + if (RT && RT->getDecl() == D) { +Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node) +<< D->getDeclKindName(); +return true; + } } } Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -989,7 +989,7 @@ " return 0;" "}", Lang_C, "input.c"); - auto FromVar = + auto *FromVar = FirstDeclMatcher().match(FromTU, varDecl(hasName("d"))); ASSERT_TRUE(FromVar); auto ToType = @@ -999,12 +999,41 @@ TEST_P(ASTImporterTestBase, ImportRecordDeclInFuncParams) { // This construct is not supported by ASTImporter. - Decl *FromTU = - getTuDecl("int declToImport(struct data_t{int a;int b;} *d){ return 0; }", -Lang_C, "input.c"); - auto From = FirstDeclMatcher().match(FromTU, functionDecl()); + Decl *FromTU = getTuDecl( + "int declToImport(struct data_t{int a;int b;} ***d){ return 0; }", + Lang_C, "input.c"); + auto *From = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("declToImport"))); + ASSERT_TRUE(From); + auto *To = Import(From, Lang_C); + EXPECT_EQ(To, nullptr); +} + +TEST_P(ASTImporterTestBase, ImportRecordDeclInFuncFromMacro) { + Decl *FromTU = getTuDecl( + "#define NONAME_SIZEOF(type) sizeof(struct{type *dummy;}) \n" + "int declToImport(){ return NONAME_SIZEOF(int); }", + Lang_C, "input.c"); + auto *From = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("declToImport"))); + ASSERT_TRUE(From); + auto *To = Import(From, Lang_C); + ASSERT_TRUE(To); + EXPECT_TRUE(MatchVerifier().match( + To, functionDecl(hasName("declToImport"), + hasDescendant(unaryExprOrTypeTraitExpr(); +} + +TEST_P(ASTImporterTestBase, ImportRecordDeclInFuncParamsFromMacro) { + // This construct is not supported by ASTImporter. + Decl *FromTU = getTuDecl( + "#define PAIR_STRUCT(type) struct data_t{type a;type b;} \n" + "int declToImport(PAIR_STRUCT(int) ***d){ return 0; }", + Lang_C, "input.c"); + auto *From = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("declToImport"))); ASSERT_TRUE(From); - auto To = Import(From, Lang_C); + auto *To = Import(From, Lang_C); EXPECT_EQ(To, nullptr); } Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -1147,15 +1147,21 @@ FunctionDecl *FunDecl; if (isa(D) && (FunDecl = dyn_cast(OrigDC)) && FunDecl->hasBody()) { -SourceRange RecR = D->getSourceRange(); -SourceRange BodyR = FunDecl->getBody()->getSourceRange(); -// If RecordDecl is not in Body (it is a param), we bail out. -if (RecR.isValid() && BodyR.isValid() && -(RecR.getBegin() < BodyR.getBegin() || - BodyR.getEnd() < RecR.getEnd())) { - Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node) - << D->getDeclKindName(); - return true; +auto getLeafPointeeType = [](const Type *T) { + while (T->isPointerType() || T->isArrayType()) { +T = T->getPoin
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong added a comment. Ping. Manuel, I still don't see how could we apply `match(anyOf(node), hasDescendant(node))` to the problem of general subtree traversal. (I'd like to have support for not just `decl()` but other kind of nodes too.) Could you please advise how to move on? Also, could you please describe your specific technical arguments against this patch? Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50428: Add support for importing imaginary literals
martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -553,6 +553,14 @@ floatLiteral(equals(1.0e-5f), hasType(asString("float")); } +TEST_P(ImportExpr, ImportImaginaryLiteralExpr) { + MatchVerifier Verifier; + testImport( + "void declToImport() { (void)1.0i; }", + Lang_CXX14, "", Lang_CXX14, Verifier, + functionDecl(hasDescendant(imaginaryLiteral(; +} + TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; testImport( Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -710,6 +710,7 @@ const internal::VariadicDynCastAllOfMatcher integerLiteral; const internal::VariadicDynCastAllOfMatcher floatLiteral; +const internal::VariadicDynCastAllOfMatcher imaginaryLiteral; const internal::VariadicDynCastAllOfMatcher userDefinedLiteral; const internal::VariadicDynCastAllOfMatcher Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -412,6 +412,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacterLiteral(CharacterLiteral *E); Expr *VisitStringLiteral(StringLiteral *E); Expr *VisitCompoundLiteralExpr(CompoundLiteralExpr *E); @@ -5594,6 +5595,7 @@ Importer.Import(E->getLocation())); } + Expr *ASTNodeImporter::VisitFloatingLiteral(FloatingLiteral *E) { QualType T = Importer.Import(E->getType()); if (T.isNull()) @@ -5604,6 +5606,19 @@ Importer.Import(E->getLocation())); } +Expr *ASTNodeImporter::VisitImaginaryLiteral(ImaginaryLiteral *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SubE = Importer.Import(E->getSubExpr()); + if (!SubE) +return nullptr; + + return new (Importer.getToContext()) + ImaginaryLiteral(SubE, T); +} + Expr *ASTNodeImporter::VisitCharacterLiteral(CharacterLiteral *E) { QualType T = Importer.Import(E->getType()); if (T.isNull()) Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -1975,6 +1975,11 @@ extern const internal::VariadicDynCastAllOfMatcher floatLiteral; +/// Matches imaginary literals, which are based on integer and floating +/// point literals e.g.: 1i, 1.0i +extern const internal::VariadicDynCastAllOfMatcher +imaginaryLiteral; + /// Matches user defined literal operator call. /// /// Example match: "foo"_suffix Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -553,6 +553,14 @@ floatLiteral(equals(1.0e-5f), hasType(asString("float")); } +TEST_P(ImportExpr, ImportImaginaryLiteralExpr) { + MatchVerifier Verifier; + testImport( + "void declToImport() { (void)1.0i; }", + Lang_CXX14, "", Lang_CXX14, Verifier, + functionDecl(hasDescendant(imaginaryLiteral(; +} + TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; testImport( Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -710,6 +710,7 @@ const internal::VariadicDynCastAllOfMatcher integerLiteral; const internal::VariadicDynCastAllOfMatcher floatLiteral; +const internal::VariadicDynCastAllOfMatcher imaginaryLiteral; const internal::VariadicDynCastAllOfMatcher userDefinedLiteral; const internal::VariadicDynCastAllOfMatcher Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -412,6 +412,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacter
[PATCH] D50428: Add support for importing imaginary literals
martong updated this revision to Diff 159662. martong added a comment. Remove superflous newline Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -553,6 +553,14 @@ floatLiteral(equals(1.0e-5f), hasType(asString("float")); } +TEST_P(ImportExpr, ImportImaginaryLiteralExpr) { + MatchVerifier Verifier; + testImport( + "void declToImport() { (void)1.0i; }", + Lang_CXX14, "", Lang_CXX14, Verifier, + functionDecl(hasDescendant(imaginaryLiteral(; +} + TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; testImport( Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -710,6 +710,7 @@ const internal::VariadicDynCastAllOfMatcher integerLiteral; const internal::VariadicDynCastAllOfMatcher floatLiteral; +const internal::VariadicDynCastAllOfMatcher imaginaryLiteral; const internal::VariadicDynCastAllOfMatcher userDefinedLiteral; const internal::VariadicDynCastAllOfMatcher Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -412,6 +412,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacterLiteral(CharacterLiteral *E); Expr *VisitStringLiteral(StringLiteral *E); Expr *VisitCompoundLiteralExpr(CompoundLiteralExpr *E); @@ -5604,6 +5605,19 @@ Importer.Import(E->getLocation())); } +Expr *ASTNodeImporter::VisitImaginaryLiteral(ImaginaryLiteral *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SubE = Importer.Import(E->getSubExpr()); + if (!SubE) +return nullptr; + + return new (Importer.getToContext()) + ImaginaryLiteral(SubE, T); +} + Expr *ASTNodeImporter::VisitCharacterLiteral(CharacterLiteral *E) { QualType T = Importer.Import(E->getType()); if (T.isNull()) Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -1975,6 +1975,11 @@ extern const internal::VariadicDynCastAllOfMatcher floatLiteral; +/// Matches imaginary literals, which are based on integer and floating +/// point literals e.g.: 1i, 1.0i +extern const internal::VariadicDynCastAllOfMatcher +imaginaryLiteral; + /// Matches user defined literal operator call. /// /// Example match: "foo"_suffix Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -553,6 +553,14 @@ floatLiteral(equals(1.0e-5f), hasType(asString("float")); } +TEST_P(ImportExpr, ImportImaginaryLiteralExpr) { + MatchVerifier Verifier; + testImport( + "void declToImport() { (void)1.0i; }", + Lang_CXX14, "", Lang_CXX14, Verifier, + functionDecl(hasDescendant(imaginaryLiteral(; +} + TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; testImport( Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -710,6 +710,7 @@ const internal::VariadicDynCastAllOfMatcher integerLiteral; const internal::VariadicDynCastAllOfMatcher floatLiteral; +const internal::VariadicDynCastAllOfMatcher imaginaryLiteral; const internal::VariadicDynCastAllOfMatcher userDefinedLiteral; const internal::VariadicDynCastAllOfMatcher Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -412,6 +412,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacterLiteral(CharacterLiteral *E); Expr *VisitStringLiteral(StringLiteral *E); Expr *VisitCompoundLiteralExpr(CompoundLiteralExpr *E); @@ -5604,6 +5605,19 @@ Importer.Import(E->getLocation())); } +Expr *ASTNodeImporter::VisitImaginaryL
[PATCH] D50444: Fix structural inequivalency of forward EnumDecl
martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently we consider one forward declared RecordDecl and another with a definition equal. We have to do the same in case of enums. Repository: rC Clang https://reviews.llvm.org/D50444 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/StructuralEquivalenceTest.cpp === --- unittests/AST/StructuralEquivalenceTest.cpp +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -618,6 +618,31 @@ EXPECT_FALSE(testStructuralMatch(R0, R1)); } +TEST_F(StructuralEquivalenceRecordTest, +FwdDeclRecordShouldBeEqualWithFwdDeclRecord) { + auto t = makeNamedDecls("class foo;", "class foo;", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + FwdDeclRecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = + makeNamedDecls("class foo;", "class foo { int A; };", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + RecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = makeNamedDecls("class foo { int A; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, RecordsWithDifferentBody) { + auto t = makeNamedDecls("class foo { int B; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) { auto t = makeNamedDecls( @@ -627,5 +652,33 @@ EXPECT_FALSE(testStructuralMatch(t)); } +struct StructuralEquivalenceEnumTest : StructuralEquivalenceTest {}; + +TEST_F(StructuralEquivalenceEnumTest, FwdDeclEnumShouldBeEqualWithFwdDeclEnum) { + auto t = makeNamedDecls("enum class foo;", "enum class foo;", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, + FwdDeclEnumShouldBeEqualWithEnumWhichHasDefinition) { + auto t = + makeNamedDecls("enum class foo;", "enum class foo { A };", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, + EnumShouldBeEqualWithEnumWhichHasDefinition) { + auto t = makeNamedDecls("enum class foo { A };", "enum class foo { A };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, EnumsWithDifferentBody) { + auto t = makeNamedDecls("enum class foo { B };", "enum class foo { A };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} + + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/ASTStructuralEquivalence.cpp === --- lib/AST/ASTStructuralEquivalence.cpp +++ lib/AST/ASTStructuralEquivalence.cpp @@ -1178,6 +1178,14 @@ /// Determine structural equivalence of two enums. static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, EnumDecl *D1, EnumDecl *D2) { + + // Compare the definitions of these two eunums. If either or both are + // incomplete (i.e. forward declared), we assume that they are equivalent. + D1 = D1->getDefinition(); + D2 = D2->getDefinition(); + if (!D1 || !D2) +return true; + EnumDecl::enumerator_iterator EC2 = D2->enumerator_begin(), EC2End = D2->enumerator_end(); for (EnumDecl::enumerator_iterator EC1 = D1->enumerator_begin(), Index: unittests/AST/StructuralEquivalenceTest.cpp === --- unittests/AST/StructuralEquivalenceTest.cpp +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -618,6 +618,31 @@ EXPECT_FALSE(testStructuralMatch(R0, R1)); } +TEST_F(StructuralEquivalenceRecordTest, +FwdDeclRecordShouldBeEqualWithFwdDeclRecord) { + auto t = makeNamedDecls("class foo;", "class foo;", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + FwdDeclRecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = + makeNamedDecls("class foo;", "class foo { int A; };", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + RecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = makeNamedDecls("class foo { int A; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, RecordsWithDifferentBody) { + auto t = makeNamedDecls("class foo { int B; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} TEST_F(StructuralEquivalenceTest, Co
[PATCH] D50451: Fix import of class templates partial specialization
martong created this revision. martong added reviewers: a_sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. 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. Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2641,6 +2641,216 @@ R1, recordDecl(has(fieldDecl(hasName("next")); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + 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 xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().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 xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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 + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x('c'); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + // Match the void(int) ctor. + auto CtorPattern = + cxxConstructorDecl(hasParameter(0, varDecl(hasType(asString("int", + hasParent(classTemplateSpecializationDecl())); + auto *FromCtor = + FirstDeclMatcher().match(FromTU, CtorPattern); + auto *ToCtor = + FirstDeclMatcher().match(ToTU, CtorPattern); + ASSERT_TRUE(FromCtor->hasBody()); + ASSERT_FALSE(ToCtor->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match(
[PATCH] D50428: [ASTImporter] Add support for importing imaginary literals
This revision was automatically updated to reflect the committed changes. Closed by commit rL339334: Add support for importing imaginary literals (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50428?vs=159662&id=159902#toc Repository: rL LLVM https://reviews.llvm.org/D50428 Files: 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 Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h === --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h @@ -1975,6 +1975,11 @@ extern const internal::VariadicDynCastAllOfMatcher floatLiteral; +/// Matches imaginary literals, which are based on integer and floating +/// point literals e.g.: 1i, 1.0i +extern const internal::VariadicDynCastAllOfMatcher +imaginaryLiteral; + /// Matches user defined literal operator call. /// /// Example match: "foo"_suffix Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -434,6 +434,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacterLiteral(CharacterLiteral *E); Expr *VisitStringLiteral(StringLiteral *E); Expr *VisitCompoundLiteralExpr(CompoundLiteralExpr *E); @@ -5613,6 +5614,18 @@ Importer.Import(E->getLocation())); } +Expr *ASTNodeImporter::VisitImaginaryLiteral(ImaginaryLiteral *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SubE = Importer.Import(E->getSubExpr()); + if (!SubE) +return nullptr; + + return new (Importer.getToContext()) ImaginaryLiteral(SubE, T); +} + Expr *ASTNodeImporter::VisitCharacterLiteral(CharacterLiteral *E) { QualType T = Importer.Import(E->getType()); if (T.isNull()) Index: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp === --- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp +++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -710,6 +710,7 @@ const internal::VariadicDynCastAllOfMatcher integerLiteral; const internal::VariadicDynCastAllOfMatcher floatLiteral; +const internal::VariadicDynCastAllOfMatcher imaginaryLiteral; const internal::VariadicDynCastAllOfMatcher userDefinedLiteral; const internal::VariadicDynCastAllOfMatcher Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -553,6 +553,14 @@ floatLiteral(equals(1.0e-5f), hasType(asString("float")); } +TEST_P(ImportExpr, ImportImaginaryLiteralExpr) { + MatchVerifier Verifier; + testImport( + "void declToImport() { (void)1.0i; }", + Lang_CXX14, "", Lang_CXX14, Verifier, + functionDecl(hasDescendant(imaginaryLiteral(; +} + TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; testImport( Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h === --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h @@ -1975,6 +1975,11 @@ extern const internal::VariadicDynCastAllOfMatcher floatLiteral; +/// Matches imaginary literals, which are based on integer and floating +/// point literals e.g.: 1i, 1.0i +extern const internal::VariadicDynCastAllOfMatcher +imaginaryLiteral; + /// Matches user defined literal operator call. /// /// Example match: "foo"_suffix Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -434,6 +434,7 @@ Expr *VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E); Expr *VisitIntegerLiteral(IntegerLiteral *E); Expr *VisitFloatingLiteral(FloatingLiteral *E); +Expr *VisitImaginaryLiteral(ImaginaryLiteral *E); Expr *VisitCharacterLiteral(CharacterLiteral *E); Expr *VisitStringLiteral(StringLiteral *E); Expr *VisitCompoundLiteralExpr(CompoundLiteralExpr *E); @@ -5613,6 +5614,18 @@ Importer.Import(E->getLocation())); } +Expr *ASTNodeImporter::VisitImaginaryLiteral(ImaginaryLiteral *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SubE = Importer.Import(E
[PATCH] D50444: [ASTImporter] Fix structural inequivalency of forward EnumDecl
This revision was automatically updated to reflect the committed changes. Closed by commit rL339336: Fix structural inequivalency of forward EnumDecl (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50444?vs=159697&id=159905#toc Repository: rL LLVM https://reviews.llvm.org/D50444 Files: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp Index: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp === --- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp +++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp @@ -1178,6 +1178,14 @@ /// Determine structural equivalence of two enums. static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, EnumDecl *D1, EnumDecl *D2) { + + // Compare the definitions of these two enums. If either or both are + // incomplete (i.e. forward declared), we assume that they are equivalent. + D1 = D1->getDefinition(); + D2 = D2->getDefinition(); + if (!D1 || !D2) +return true; + EnumDecl::enumerator_iterator EC2 = D2->enumerator_begin(), EC2End = D2->enumerator_end(); for (EnumDecl::enumerator_iterator EC1 = D1->enumerator_begin(), Index: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp === --- cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp +++ cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp @@ -642,13 +642,67 @@ EXPECT_FALSE(testStructuralMatch(t)); } +TEST_F(StructuralEquivalenceRecordTest, +FwdDeclRecordShouldBeEqualWithFwdDeclRecord) { + auto t = makeNamedDecls("class foo;", "class foo;", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + FwdDeclRecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = + makeNamedDecls("class foo;", "class foo { int A; };", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, + RecordShouldBeEqualWithRecordWhichHasDefinition) { + auto t = makeNamedDecls("class foo { int A; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceRecordTest, RecordsWithDifferentBody) { + auto t = makeNamedDecls("class foo { int B; };", "class foo { int A; };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} + TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) { auto t = makeNamedDecls( "struct A{ }; struct B{ }; void foo(A a, A b);", "struct A{ }; struct B{ }; void foo(A a, B b);", Lang_CXX); EXPECT_FALSE(testStructuralMatch(t)); } +struct StructuralEquivalenceEnumTest : StructuralEquivalenceTest {}; + +TEST_F(StructuralEquivalenceEnumTest, FwdDeclEnumShouldBeEqualWithFwdDeclEnum) { + auto t = makeNamedDecls("enum class foo;", "enum class foo;", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, + FwdDeclEnumShouldBeEqualWithEnumWhichHasDefinition) { + auto t = + makeNamedDecls("enum class foo;", "enum class foo { A };", Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, + EnumShouldBeEqualWithEnumWhichHasDefinition) { + auto t = makeNamedDecls("enum class foo { A };", "enum class foo { A };", + Lang_CXX11); + EXPECT_TRUE(testStructuralMatch(t)); +} + +TEST_F(StructuralEquivalenceEnumTest, EnumsWithDifferentBody) { + auto t = makeNamedDecls("enum class foo { B };", "enum class foo { A };", + Lang_CXX11); + EXPECT_FALSE(testStructuralMatch(t)); +} + + } // end namespace ast_matchers } // end namespace clang Index: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp === --- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp +++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp @@ -1178,6 +1178,14 @@ /// Determine structural equivalence of two enums. static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, EnumDecl *D1, EnumDecl *D2) { + + // Compare the definitions of these two enums. If either or both are + // incomplete (i.e. forward declared), we assume that they are equivalent. + D1 = D1->getDefinition(); + D2 = D2->getDefinition(); + if (!D1 || !D2) +return true; + EnumDecl::enumerator_iterator EC2 = D2->enumerator_begin(), EC2End = D2->enumerator_end(); for (EnumDecl::enumerator_iterator EC1 = D1->enumerator_begin(), Index: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp === --- cfe/trunk/unittests/AS
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong marked 5 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2872 Importer.MapImported(D, FoundField); +// In case of a FieldDecl of a ClassTemplateSpecializationDecl, the +// initializer of a FieldDecl might not had been instantiated in the a_sidorin wrote: > Honestly speaking, I wonder about this behaviour because it doesn't look > similar to instantiation of only methods that are used. Is it a common rule? Yes, this is a common rule. The instantiation of an initializer is similar to the instantiation of default arguments in a sense that both are instantated only if they are being used. To be more precise, quoting from Vandevoorde - C++ Templates The Complete Guide / 14.2.2 Instantiated Components: ... Default functioncallarguments are considered separately wheninstantiating templates. Specifically, theyare not instantiatedunless there is a callto that function(or member function) thatactuallymakes use of the default argument. If, on the other hand, the functionis called withexplicitarguments thatoverride the default,thenthe default arguments are not instantiated. Similarly, exception specifications and **default member initializers** are not instantiatedunless theyare needed. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); a_sidorin wrote: > Importing additional fields can change the layout of the specialization. For > CSA, this usually results in strange assertions hard to debug. Could you > please share the results of testing of this change? > This change also doesn't seem to have related tests in this patch. TLDR; We will not create additional fields. By the time when we import the field, we already know that the existing specialization is structurally equivalent with the new one. Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, the structural equivalence check ensures that they have the exact same fields. When we import the field of the new spec and if there is an existing FieldDecl in the "To" context, then no new FieldDecl will be created (this is handled in `VisitFieldDecl` by first doing a lookup of existing field with the same name and type). This patch extends `VisitFieldDecl` in a way that we add new initializer expressions to the existing FieldDecl, if it didn't have and in the "From" context it has. For the record, I added a new test to make sure that a new FieldDecl will not be created during the merge. Comment at: lib/AST/ASTImporter.cpp:4551 + for (auto *FromField : D->fields()) +Importer.Import(FromField); + a_sidorin wrote: > The result of import is unchecked here and below. Is it intentional? Yes, that is intentional. We plan to refactor all ASTImporter functions to provide a proper error handling mechanism, and in that change we would like to enforce the check of all import functions. Unfortunately, currently we already have many places where we do not check the return value of import. Repository: rC Clang https://reviews.llvm.org/D50451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong marked 3 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4551 + for (auto *FromField : D->fields()) +Importer.Import(FromField); + martong wrote: > a_sidorin wrote: > > The result of import is unchecked here and below. Is it intentional? > Yes, that is intentional. We plan to refactor all ASTImporter functions to > provide a proper error handling mechanism, and in that change we would like > to enforce the check of all import functions. > Unfortunately, currently we already have many places where we do not check > the return value of import. Actually, having proper error handling is just one thing, the other more important reason why we don't check the result of the import here, because we don't want to stop merging other fields/functions if one merge failed. If one merge failed, other merge operations still could be successful. Repository: rC Clang https://reviews.llvm.org/D50451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong updated this revision to Diff 160552. martong added a comment. - Add new test - Fix indentation Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2683,6 +2683,263 @@ EXPECT_EQ(FromIndex, 3u); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + 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 xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().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 xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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 + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter().match( +ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"(
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); martong wrote: > a_sidorin wrote: > > Importing additional fields can change the layout of the specialization. > > For CSA, this usually results in strange assertions hard to debug. Could > > you please share the results of testing of this change? > > This change also doesn't seem to have related tests in this patch. > TLDR; We will not create additional fields. > > By the time when we import the field, we already know that the existing > specialization is structurally equivalent with the new one. > Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, the > structural equivalence check ensures that they have the exact same fields. > When we import the field of the new spec and if there is an existing > FieldDecl in the "To" context, then no new FieldDecl will be created (this is > handled in `VisitFieldDecl` by first doing a lookup of existing field with > the same name and type). > This patch extends `VisitFieldDecl` in a way that we add new initializer > expressions to the existing FieldDecl, if it didn't have and in the "From" > context it has. > > For the record, I added a new test to make sure that a new FieldDecl will > not be created during the merge. This is the new test: `ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks that it is not possible to add new fields to a specialization, rather an ODR violation is diagnosed. Repository: rC Clang https://reviews.llvm.org/D50451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1317 + for (auto *D : FromRD->decls()) { +Decl *ToD = Importer.GetAlreadyImportedOrNull(D); +assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD)); Is it sure that `ToD` will never be a nullptr? I think, `removeDecl` or `addDeclInternal` below may crash if we call it with a nullptr. Also in the assert, `ToD->getDeclContext()` seems achy if `ToD` is a nullptr. Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. Hi Aleksei, Thank you for this patch. With Balazs, we are working on something similar, but with a different, fine grained error value mechanism. Unfortunately we were not aware of that you have been working on error handling, and we didn't say that we are working on error handling recently, I am terribly sorry about this. From the CSA perspective, we realized that there may be several different error cases which has to be handled differently in `CrossTranslationUnit.cpp`. For example, there are unsupported constructs which we do not support to import (like a struct definition as a parameter of a function). Another example is when there is a name conflict between the decl in the "From" context and the decl in the "To" context, this usually means an ODR error. We have to handle these errors in a different way after we imported one function during CTU analysis. The fact that there may be more than one kind of errors yields for the use of the designated LLVM types: `Error` and `Expected`. A simple `Optional` is probably not generic enough. I find the `importNonNull` and generally the new family of `import` functions useful, but I am not sure how they could cooperate with `Expected`. Especially, I have some concerns with output parameters. If we have an Expected as a result type, then there is no way to acquire the T if there was an error. However, when we have output parameters, then even if there was an error some output params could have been set ... and those can be reused even after the return. If error handling is not proper on the callee site then we may continue with stale values, which is not possible if we use Expected as a return value. Do you think we can hold back this patch for a few days until Balazs prepares the `Expected` based version? Then I think we could compare the patches and we could merge the best from the two of them. Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. > Do you think we can hold back this patch for a few days until Balazs prepares > the Expected based version? Then I think we could compare the patches and > we could merge the best from the two of them. I mean, once Balazs is also ready, then we can create a combined patch which holds the good things from both of yours and his patches. Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. Roughly, this is the direction where we are heading with `Expected`: https://github.com/Ericsson/clang/pull/457/commits/783b7f5cce9de589a5c3c3ae983398cf499077ec (If you are interested, here is our whole thought process: https://github.com/Ericsson/clang/pull/457) Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
martong accepted this revision. martong added inline comments. This revision is now accepted and ready to land. Comment at: lib/AST/ASTImporter.cpp:1317 + for (auto *D : FromRD->decls()) { +Decl *ToD = Importer.GetAlreadyImportedOrNull(D); +assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD)); a_sidorin wrote: > martong wrote: > > Is it sure that `ToD` will never be a nullptr? > > I think, `removeDecl` or `addDeclInternal` below may crash if we call it > > with a nullptr. > > Also in the assert, `ToD->getDeclContext()` seems achy if `ToD` is a > > nullptr. > We have an early return if such import failed before (line 1300). Okay, that line did not catch my attention. Could you please provide a comment which describes that at this point we expect ToD to be non-null, or even better, we could have an explicit `assert(ToD) `. Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50792: [ASTImporter] Add test for member pointer types.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D50792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50793: [ASTImporter] Add test for importing CompoundAssignOperators
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Looks good. Repository: rC Clang https://reviews.llvm.org/D50793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50810: [ASTImporter] Add test for DoStmt
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D50810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50811: [ASTImporter] Add test for WhileStmt
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D50811 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50813: [ASTImporter] Add test for IndirectGotoStmt
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D50813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. > Yes, I was thinking about it too. The reason why I chose Optional was that > ASTImporter clients don't use the error kind. If you have any plans on > changing this, Expected is a preferred approach. Yes, we plan to submit a patch to LLDB too which would apply the use of Expected. > If I understand you correctly, importNonNull and importNullable should work > with Expected pretty well. Yes, that's right. > Changing import*Into return result to Error can partially help in avoiding > usage of an unchecked result. These functions already guarantee that their > parameters are changed only if the internal import was successful. Yes I agree, that `Error` can help, and I also agree that this help is just partial. If we change `importNonNullInto` to has an `Error` return value template LLVM_NODISCARD Error importNonNullInto(DeclTy *&ToD, Decl *FromD) { if (auto Res = Importer.Import(FromD)) { ToD = cast(*Res); return make_error(); } return Error::success(); } then on the call site, on the branch where whe handle the error we may make mistakes like this: CXXRecordDecl *ToTemplated = nullptr; if (Err = importNonNullInto(ToTemplated, FromTemplated)) { foo(ToTemplated->hasDefinition()); // Undefined Behaviour! return Err; } If we do not use output parameters, only `Expected`, then this could not happen. `Expected.get()` aborts if there was an error. Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong updated this revision to Diff 161698. martong marked an inline comment as done. martong added a comment. - Change comments Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2683,6 +2683,263 @@ EXPECT_EQ(FromIndex, 3u); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + 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 xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().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 xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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 + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter().match( +ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = getToTuDecl
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong marked an inline comment as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); a_sidorin wrote: > martong wrote: > > martong wrote: > > > a_sidorin wrote: > > > > Importing additional fields can change the layout of the > > > > specialization. For CSA, this usually results in strange assertions > > > > hard to debug. Could you please share the results of testing of this > > > > change? > > > > This change also doesn't seem to have related tests in this patch. > > > TLDR; We will not create additional fields. > > > > > > By the time when we import the field, we already know that the existing > > > specialization is structurally equivalent with the new one. > > > Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, > > > the structural equivalence check ensures that they have the exact same > > > fields. > > > When we import the field of the new spec and if there is an existing > > > FieldDecl in the "To" context, then no new FieldDecl will be created > > > (this is handled in `VisitFieldDecl` by first doing a lookup of existing > > > field with the same name and type). > > > This patch extends `VisitFieldDecl` in a way that we add new initializer > > > expressions to the existing FieldDecl, if it didn't have and in the > > > "From" context it has. > > > > > > For the record, I added a new test to make sure that a new FieldDecl > > > will not be created during the merge. > > This is the new test: > > `ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks > > that it is not possible to add new fields to a specialization, rather an > > ODR violation is diagnosed. > Thank you for the explanation. However, I find the comment very misleading. > It tells: > ``` > // Check and merge those fields which have been instantiated > // in the "From" context, but not in the "To" context. > ``` > Would it be correct to change it to "Import field initializers that are still > not instantiated", or do I still misunderstand something? Yes, I agree that this comment is not precise and could be misleading, thank you for pointing this out. I changed this comment and others too. Repository: rC Clang https://reviews.llvm.org/D50451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong updated this revision to Diff 161699. martong added a comment. - Add one more TODO to import instantiated exception specifications Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2683,6 +2683,263 @@ EXPECT_EQ(FromIndex, 3u); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + 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 xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().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 xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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 + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter().match( +ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = get
[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D51001 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
This revision was automatically updated to reflect the committed changes. Closed by commit rL340402: Fix import of class templates partial specialization (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50451?vs=161699&id=161923#toc Repository: rL LLVM https://reviews.llvm.org/D50451 Files: 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 Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -2955,19 +2955,276 @@ auto *FromD = FirstDeclMatcher().match( FromTu, classTemplateDecl(hasName("declToImport"))); auto *ToD = Import(FromD, Lang_CXX); - + auto Pattern = classTemplateDecl( has(cxxRecordDecl(has(friendDecl(has(classTemplateDecl())); ASSERT_TRUE(MatchVerifier{}.match(FromD, Pattern)); EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern)); - + auto *Class = FirstDeclMatcher().match(ToD, classTemplateDecl()); auto *Friend = FirstDeclMatcher().match(ToD, friendDecl()); EXPECT_NE(Friend->getFriendDecl(), Class); EXPECT_EQ(Friend->getFriendDecl()->getPreviousDecl(), Class); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + 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 xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().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 xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().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 + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11
[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D46398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong created this revision. martong added reviewers: xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. `DeclContext` is essentially a list of `Decl`s and a lookup table (`LookupPtr`) but these are encapsulated. E.g. `LookupPtr` is private. `DeclContext::removeDecl` has the responsibility to remove the given decl from the list and from the table too. Template specializations cannot participate in normal (qualified) lookup. Consequently no template specialization can be in the lookup table, but they will be in the list. When the lookup table is built or when a new `Decl` is added, then it is checked whether it is a template specialization and if so it is skipped from the lookup table. Thus, whenever we want to remove a `Decl` from a `DeclContext` we must not reach the point to remove that from the lookup table (and that is what this patch do). With respect to ASTImporter: At some point probably we will be reordering `FriendDecl`s and possibly `CXXMethodDecl`s in a `RecordDecl` at `importDeclContext` to be able to structurally compare two `RecordDecl`s. When that happens we most probably want to remove all `Decls` from a `RecordDecl` and then we would add them in the proper order. Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/MatchVerifier.h Index: unittests/AST/MatchVerifier.h === --- unittests/AST/MatchVerifier.h +++ unittests/AST/MatchVerifier.h @@ -28,11 +28,12 @@ namespace clang { namespace ast_matchers { -enum Language { +enum Language { Lang_C, Lang_C89, Lang_CXX, Lang_CXX11, +Lang_CXX14, Lang_OpenCL, Lang_OBJCXX }; @@ -113,6 +114,10 @@ Args.push_back("-std=c++11"); FileName = "input.cc"; break; + case Lang_CXX14: +Args.push_back("-std=c++14"); +FileName = "input.cc"; +break; case Lang_OpenCL: FileName = "input.cl"; break; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -52,6 +52,9 @@ case Lang_CXX11: BasicArgs = {"-std=c++11", "-frtti"}; break; + case Lang_CXX14: +BasicArgs = {"-std=c++14", "-frtti"}; +break; case Lang_OpenCL: case Lang_OBJCXX: llvm_unreachable("Not implemented yet!"); @@ -1764,5 +1767,86 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +TEST(ImportExpr, ImportClassTemplatePartialSpecialization) { + MatchVerifier Verifier; + auto Code = +R"s( +struct declToImport { + template + struct X {}; + template + struct X {}; + X x0; +}; +)s"; + + testImport(Code, Lang_CXX, "", Lang_CXX, Verifier, recordDecl()); +} + +TEST(ImportExpr, ImportSpecializationsOfFriendClassTemplate) { + MatchVerifier Verifier; + auto Code = +R"( +namespace declToImport { + +template +struct F{}; + +class X { + friend struct F; + friend struct F; +}; + +} // namespace +)"; + + testImport(Code, Lang_CXX11, "", Lang_CXX11, Verifier, namespaceDecl()); +} + +TEST(ImportExpr, ImportSpecializationsOfVariableTemplate) { + MatchVerifier Verifier; + auto Code = +R"( +namespace declToImport { + +struct limits { + template + static const T min; // declaration of a static data member template +}; +template +const T limits::min = { }; // definition of a static data member template + +template const int limits::min; // explicit instantiation + +} // namespace +)"; + + testImport(Code, Lang_CXX14, "", Lang_CXX14, Verifier, namespaceDecl()); +} + +TEST_P(ASTImporterTestBase, DISABLED_ImportOfRecordWithDifferentFriends) { + Decl *ToR1; + { +Decl *FromTU = getTuDecl("struct A { friend class F0; friend class F1; };", + Lang_CXX, "input0.cc"); +auto *FromR = FirstDeclMatcher().match( +FromTU, cxxRecordDecl(hasName("A"))); + +ToR1 = Import(FromR, Lang_CXX); + } + + Decl *ToR2; + { +Decl *FromTU = +getTuDecl("struct A { friend class F0; };", Lang_CXX, "input1.cc"); +auto *FromR = FirstDeclMatcher().match( +FromTU, cxxRecordDecl(hasName("A"))); + +ToR2 = Import(FromR, Lang_CXX); + } + + EXPECT_NE(ToR1, ToR2); +} + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/DeclBase.cpp === --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,8 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +static bool shouldBeHidden(NamedDecl *D); + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,14 +1374,18 @@ }
[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates
martong updated this revision to Diff 146765. martong added a comment. - Add FIXME Repository: rC Clang https://reviews.llvm.org/D46353 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1431,6 +1431,39 @@ MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern)); } +TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) { + { +Decl *FromTU = getTuDecl( +R"( +template +struct B; +)", +Lang_CXX, "input0.cc"); +auto *FromD = FirstDeclMatcher().match( +FromTU, classTemplateDecl(hasName("B"))); + +Import(FromD, Lang_CXX); + } + + { +Decl *FromTU = getTuDecl( +R"( +template +struct B { + void f(); +}; +)", +Lang_CXX, "input1.cc"); +FunctionDecl *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +Import(FromD, Lang_CXX); +auto *FromCTD = FirstDeclMatcher().match( +FromTU, classTemplateDecl(hasName("B"))); +auto *ToCTD = cast(Import(FromCTD, Lang_CXX)); +EXPECT_TRUE(ToCTD->isThisDeclarationADefinition()); + } +} + INSTANTIATE_TEST_CASE_P( ParameterizedTests, ASTImporterTestBase, ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4077,8 +4077,14 @@ if (auto *FoundTemplate = dyn_cast(Found)) { if (IsStructuralMatch(D, FoundTemplate)) { // The class templates structurally match; call it the same template. - // FIXME: We may be filling in a forward declaration here. Handle - // this case! + + // We found a forward declaration but the class to be imported has a + // definition. + // FIXME Add this forward declaration to the redeclaration chain. + if (D->isThisDeclarationADefinition() && + !FoundTemplate->isThisDeclarationADefinition()) +continue; + Importer.Imported(D->getTemplatedDecl(), FoundTemplate->getTemplatedDecl()); return Importer.Imported(D, FoundTemplate); Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1431,6 +1431,39 @@ MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern)); } +TEST_P(ASTImporterTestBase, ImportDefinitionOfClassTemplateAfterFwdDecl) { + { +Decl *FromTU = getTuDecl( +R"( +template +struct B; +)", +Lang_CXX, "input0.cc"); +auto *FromD = FirstDeclMatcher().match( +FromTU, classTemplateDecl(hasName("B"))); + +Import(FromD, Lang_CXX); + } + + { +Decl *FromTU = getTuDecl( +R"( +template +struct B { + void f(); +}; +)", +Lang_CXX, "input1.cc"); +FunctionDecl *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +Import(FromD, Lang_CXX); +auto *FromCTD = FirstDeclMatcher().match( +FromTU, classTemplateDecl(hasName("B"))); +auto *ToCTD = cast(Import(FromCTD, Lang_CXX)); +EXPECT_TRUE(ToCTD->isThisDeclarationADefinition()); + } +} + INSTANTIATE_TEST_CASE_P( ParameterizedTests, ASTImporterTestBase, ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4077,8 +4077,14 @@ if (auto *FoundTemplate = dyn_cast(Found)) { if (IsStructuralMatch(D, FoundTemplate)) { // The class templates structurally match; call it the same template. - // FIXME: We may be filling in a forward declaration here. Handle - // this case! + + // We found a forward declaration but the class to be imported has a + // definition. + // FIXME Add this forward declaration to the redeclaration chain. + if (D->isThisDeclarationADefinition() && + !FoundTemplate->isThisDeclarationADefinition()) +continue; + Importer.Imported(D->getTemplatedDecl(), FoundTemplate->getTemplatedDecl()); return Importer.Imported(D, FoundTemplate); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates
martong added a comment. Hi Aleksei, Added the FIXME, can you help me with committing this? Repository: rC Clang https://reviews.llvm.org/D46353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs, mgorny. This patch add new tests for structural equivalence. For that a new common header is created which holds the test related language specific types and functions. Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt unittests/AST/Language.h unittests/AST/MatchVerifier.h unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/StructuralEquivalenceTest.cpp === --- /dev/null +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -0,0 +1,211 @@ +#include "clang/AST/ASTContext.h" +#include "clang/AST/ASTImporter.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/AST/ASTStructuralEquivalence.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Tooling/Tooling.h" + +#include "Language.h" +#include "DeclMatcher.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace ast_matchers { + +struct StructuralEquivalenceTest : ::testing::Test { + std::unique_ptr AST0, AST1; + std::string Code0, Code1; // Buffers for SourceManager + + // Get a pair of Decl pointers to the synthetised declarations from the given + // code snipets. By default we search for the unique Decl with name 'foo' in + // both snippets. + std::tuple + makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1, + Language Lang, const char *const Identifier = "foo") { + +this->Code0 = SrcCode0; +this->Code1 = SrcCode1; +ArgVector Args = getBasicRunOptionsForLanguage(Lang); + +const char *const InputFileName = "input.cc"; + +AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName); +AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName); + +ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext(); + +auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * { + IdentifierInfo *ImportedII = &Ctx.Idents.get(Name); + assert(ImportedII && "Declaration with the identifier " + "should be specified in test!"); + DeclarationName ImportDeclName(ImportedII); + SmallVector FoundDecls; + Ctx.getTranslationUnitDecl()->localUncachedLookup(ImportDeclName, +FoundDecls); + + // We should find one Decl but one only + assert(FoundDecls.size() > 0); + assert(FoundDecls.size() < 2); + + return FoundDecls[0]; +}; + +NamedDecl *d0 = getDecl(Ctx0, Identifier); +NamedDecl *d1 = getDecl(Ctx1, Identifier); +assert(d0); +assert(d1); +return std::make_tuple(d0, d1); + } + + bool testStructuralMatch(NamedDecl *d0, NamedDecl *d1) { +llvm::DenseSet> NonEquivalentDecls; +StructuralEquivalenceContext Ctx(d0->getASTContext(), d1->getASTContext(), + NonEquivalentDecls, false, false); +return Ctx.IsStructurallyEquivalent(d0, d1); + } +}; + +using std::get; + +TEST_F(StructuralEquivalenceTest, Int) { + auto t = makeNamedDecls("int foo;", "int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedInt) { + auto t = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, Char) { + auto t = makeNamedDecls("char foo;", "char foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +// This test is disabled for now. +// FIXME Whether this is equivalent is dependendant on the target. +TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) { + auto t = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) { + auto t = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) { + auto t = makeNamedDecls("struct foo { int x; };", + "struct foo { signed int x; };", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) { + auto t = makeNamedDecls("struct foo { char x; };", + "struct foo { signed char x; };", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(get<0>(t), get<1>(t))); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) { + auto t = makeNamedDecls( + "template struct foo; template<> struct foo{};", + "template struct foo; template<> struct foo{};", + Lang_CXX); + ClassTemplateSpecializationDecl *Spec0 = + *cast(get<0>(t))->spec_beg
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong added a comment. Hi Aleksei, Thanks for reviewing this. I could synthesize a test which exercises only the `DeclContext::removeDecl` function. This test causes an assertion without the fix. Removed the rest of the testcases, which are not strictly connected to this change. Comment at: unittests/AST/ASTImporterTest.cpp:1827 + +TEST_P(ASTImporterTestBase, DISABLED_ImportOfRecordWithDifferentFriends) { + Decl *ToR1; a.sidorin wrote: > For this change, we should create a separate patch. This test is disabled ATM, but I agree it would be better to bring this in when we fix the import of friends. Repository: rC Clang https://reviews.llvm.org/D46835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong updated this revision to Diff 146845. martong marked 4 inline comments as done. martong added a comment. - Add test for removeDecl, remove unrelated tests Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/MatchVerifier.h Index: unittests/AST/MatchVerifier.h === --- unittests/AST/MatchVerifier.h +++ unittests/AST/MatchVerifier.h @@ -28,11 +28,12 @@ namespace clang { namespace ast_matchers { -enum Language { +enum Language { Lang_C, Lang_C89, Lang_CXX, Lang_CXX11, +Lang_CXX14, Lang_OpenCL, Lang_OBJCXX }; @@ -113,6 +114,10 @@ Args.push_back("-std=c++11"); FileName = "input.cc"; break; + case Lang_CXX14: +Args.push_back("-std=c++14"); +FileName = "input.cc"; +break; case Lang_OpenCL: FileName = "input.cl"; break; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -52,6 +52,9 @@ case Lang_CXX11: BasicArgs = {"-std=c++11", "-frtti"}; break; + case Lang_CXX14: +BasicArgs = {"-std=c++14", "-frtti"}; +break; case Lang_OpenCL: case Lang_OBJCXX: llvm_unreachable("Not implemented yet!"); @@ -1764,5 +1767,38 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +struct DeclContextTest : ASTImporterTestBase {}; + +TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { + Decl *TU = getTuDecl( + R"( + namespace NS { + + template + struct S {}; + template struct S; + + inline namespace INS { +template +struct S {}; +template struct S; + } + + } + )", Lang_CXX11, "input0.cc"); + auto *NS = FirstDeclMatcher().match( + TU, namespaceDecl()); + auto *Spec = FirstDeclMatcher().match( + TU, classTemplateSpecializationDecl()); + ASSERT_TRUE(NS->containsDecl(Spec)); + + NS->removeDecl(Spec); + EXPECT_FALSE(NS->containsDecl(Spec)); +} + +INSTANTIATE_TEST_CASE_P( +ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/DeclBase.cpp === --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,8 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +static bool shouldBeHidden(NamedDecl *D); + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,14 +1374,18 @@ } } } - + // Mark that D is no longer in the decl chain. D->NextInContextAndBits.setPointer(nullptr); // Remove D from the lookup table if necessary. if (isa(D)) { auto *ND = cast(D); +// Do not try to remove the declaration if that is invisible to qualified +// lookup. E.g. template specializations are skipped. +if (shouldBeHidden(ND)) return; + // Remove only decls that have a name if (!ND->getDeclName()) return; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong updated this revision to Diff 146847. martong added a comment. - Remove unrelated CXX14 changes Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/MatchVerifier.h Index: unittests/AST/MatchVerifier.h === --- unittests/AST/MatchVerifier.h +++ unittests/AST/MatchVerifier.h @@ -28,7 +28,7 @@ namespace clang { namespace ast_matchers { -enum Language { +enum Language { Lang_C, Lang_C89, Lang_CXX, Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1764,5 +1764,38 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +struct DeclContextTest : ASTImporterTestBase {}; + +TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { + Decl *TU = getTuDecl( + R"( + namespace NS { + + template + struct S {}; + template struct S; + + inline namespace INS { +template +struct S {}; +template struct S; + } + + } + )", Lang_CXX11, "input0.cc"); + auto *NS = FirstDeclMatcher().match( + TU, namespaceDecl()); + auto *Spec = FirstDeclMatcher().match( + TU, classTemplateSpecializationDecl()); + ASSERT_TRUE(NS->containsDecl(Spec)); + + NS->removeDecl(Spec); + EXPECT_FALSE(NS->containsDecl(Spec)); +} + +INSTANTIATE_TEST_CASE_P( +ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/DeclBase.cpp === --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,8 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +static bool shouldBeHidden(NamedDecl *D); + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,14 +1374,18 @@ } } } - + // Mark that D is no longer in the decl chain. D->NextInContextAndBits.setPointer(nullptr); // Remove D from the lookup table if necessary. if (isa(D)) { auto *ND = cast(D); +// Do not try to remove the declaration if that is invisible to qualified +// lookup. E.g. template specializations are skipped. +if (shouldBeHidden(ND)) return; + // Remove only decls that have a name if (!ND->getDeclName()) return; Index: unittests/AST/MatchVerifier.h === --- unittests/AST/MatchVerifier.h +++ unittests/AST/MatchVerifier.h @@ -28,7 +28,7 @@ namespace clang { namespace ast_matchers { -enum Language { +enum Language { Lang_C, Lang_C89, Lang_CXX, Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1764,5 +1764,38 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +struct DeclContextTest : ASTImporterTestBase {}; + +TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { + Decl *TU = getTuDecl( + R"( + namespace NS { + + template + struct S {}; + template struct S; + + inline namespace INS { +template +struct S {}; +template struct S; + } + + } + )", Lang_CXX11, "input0.cc"); + auto *NS = FirstDeclMatcher().match( + TU, namespaceDecl()); + auto *Spec = FirstDeclMatcher().match( + TU, classTemplateSpecializationDecl()); + ASSERT_TRUE(NS->containsDecl(Spec)); + + NS->removeDecl(Spec); + EXPECT_FALSE(NS->containsDecl(Spec)); +} + +INSTANTIATE_TEST_CASE_P( +ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/DeclBase.cpp === --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,8 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +static bool shouldBeHidden(NamedDecl *D); + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,14 +1374,18 @@ } } } - + // Mark that D is no longer in the decl chain. D->NextInContextAndBits.setPointer(nullptr); // Remove D from the lookup table if necessary. if (isa(D)) { auto *ND = cast(D); +// Do not try to remove the declaration if that is invisible to qualified +// lookup. E.g. template specializations are s
[PATCH] D46950: Fix duplicate class template definitions problem
martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. We fail to import a `ClassTemplateDecl` if the "To" context already contains a definition and then a forward decl. This is because `localUncachedLookup` does not find the definition. This is not a lookup error, the parser behaves differently than assumed in the importer code. A `DeclContext` contains one DenseMap (`LookupPtr`) which maps names to lists. The list is a special list `StoredDeclsList` which is optimized to have one element. During building the initial AST, the parser first adds the definition to the `DeclContext`. Then during parsing the second declaration (the forward decl) the parser again calls `DeclContext::addDecl` but that will not add a new element to the `StoredDeclsList` rarther it simply overwrites the old element with the most recent one. This patch fixes the error by finding the definition in the redecl chain. Added tests for the same issue with `CXXRecordDecl` and with `ClassTemplateSpecializationDecl`. These tests pass and they pass because in `VisitRecordDecl` and in `VisitClassTemplateSpecializationDecl` we already use `D->getDefinition()` after the lookup. Repository: rC Clang https://reviews.llvm.org/D46950 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index: unittests/AST/DeclMatcher.h === --- unittests/AST/DeclMatcher.h +++ unittests/AST/DeclMatcher.h @@ -44,24 +44,35 @@ using FirstDeclMatcher = DeclMatcher; template -class DeclCounter : public MatchFinder::MatchCallback { - unsigned Count = 0; +class DeclCounterWithPredicate : public MatchFinder::MatchCallback { + using UnaryPredicate = std::function; + UnaryPredicate predicate; + unsigned count = 0; void run(const MatchFinder::MatchResult &Result) override { - if(Result.Nodes.getNodeAs("")) { -++Count; - } +if (auto N = Result.Nodes.getNodeAs("")) { + if (predicate(N)) +++count; +} } + public: - // Returns the number of matched nodes under the tree rooted in `D`. + DeclCounterWithPredicate() + : predicate([](const NodeType *) { return true; }) {} + DeclCounterWithPredicate(UnaryPredicate p) : predicate(p) {} + // Returns the number of matched nodes which satisfy the predicate under the + // tree rooted in `D`. template unsigned match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; Finder.addMatcher(AMatcher.bind(""), this); Finder.matchAST(D->getASTContext()); -return Count; +return count; } }; +template +using DeclCounter = DeclCounterWithPredicate; + } // end namespace ast_matchers } // end namespace clang Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -281,8 +281,9 @@ return std::make_tuple(*FoundDecls.begin(), Imported); } - // Creates a TU decl for the given source code. - // May be called several times in a given test. + // Creates a TU decl for the given source code which can be used as a From + // context. May be called several times in a given test (with different file + // name). TranslationUnitDecl *getTuDecl(StringRef SrcCode, Language Lang, StringRef FileName = "input.cc") { assert( @@ -297,6 +298,16 @@ return Tu.TUDecl; } + // Creates the To context with the given source code and returns the TU decl. + TranslationUnitDecl *getToTuDecl(StringRef ToSrcCode, Language ToLang) { +ArgVector ToArgs = getArgVectorForLanguage(ToLang); +ToCode = ToSrcCode; +assert(!ToAST); +ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName); + +return ToAST->getASTContext().getTranslationUnitDecl(); + } + // Import the given Decl into the ToCtx. // May be called several times in a given test. // The different instances of the param From may have different ASTContext. @@ -1464,6 +1475,120 @@ } } +TEST_P(ASTImporterTestBase, + ImportDefinitionOfClassTemplateIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + template + struct B { +void f(); + }; + + template + struct B; + )", + Lang_CXX); + ASSERT_EQ(1u, DeclCounterWithPredicate( +[](const ClassTemplateDecl *T) { + return T->isThisDeclarationADefinition(); +}) +.match(ToTU, classTemplateDecl())); + + Decl *FromTU = getTuDecl( + R"( + template + struct B { +void f(); + }; + )", + Lang_CXX, "input1.cc"); + ClassTemplateDecl *FromD = FirstDeclMatcher().match( + FromTU, classTemplateDecl(hasName("B"))); + + Import(FromD, L
[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl
martong created this revision. martong added reviewers: xazax.hun, a.sidorin, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Implicit CXXRecordDecl is not added to its DeclContext during import, but in the original AST it is. This patch fixes this. Repository: rC Clang https://reviews.llvm.org/D46958 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1348,7 +1348,7 @@ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}; } -TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) { +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2110,7 +2110,7 @@ D2 = D2CXX; D2->setAccess(D->getAccess()); D2->setLexicalDeclContext(LexicalDC); - if (!DCXX->getDescribedClassTemplate()) + if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit()) LexicalDC->addDeclInternal(D2); Importer.Imported(D, D2); Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1348,7 +1348,7 @@ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}; } -TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) { +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2110,7 +2110,7 @@ D2 = D2CXX; D2->setAccess(D->getAccess()); D2->setLexicalDeclContext(LexicalDC); - if (!DCXX->getDescribedClassTemplate()) + if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit()) LexicalDC->addDeclInternal(D2); Importer.Imported(D, D2); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
martong added a comment. Hi Aleksei, I am OK with this, I just have a little concern about friend declarations. Since https://reviews.llvm.org/D32947 ( [ASTImporter] FriendDecl importing improvements ) records' structural equivalency depends on the order of their friend declarations. Anyway, I think friend related issues can be addressed in a separate patch. What is a bit more concerning is that `*ToField` can be null and we should handle that. This could happen if lookup finds the type of the field, but for some reason (e.g. there is an error in structural eq, or the redecl chain is incorrect) it turns out to be non-equivalent with the type of the FromField: for (auto *FoundDecl : FoundDecls) { if (auto *FoundField = dyn_cast(FoundDecl)) { // For anonymous fields, match up by index. if (!Name && getFieldIndex(D) != getFieldIndex(FoundField)) continue; if (Importer.IsStructurallyEquivalent(D->getType(), FoundField->getType())) { Importer.Imported(D, FoundField); return FoundField; } Importer.ToDiag(Loc, diag::err_odr_field_type_inconsistent) << Name << D->getType() << FoundField->getType(); Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here) << FoundField->getType(); return nullptr; } } Comment at: lib/AST/ASTImporter.cpp:1025 + // type import can depend on them. + const RecordDecl *FromRD = dyn_cast(FromDC); + if (!FromRD) Could use `auto` here, to avoid redundant type specification. Comment at: lib/AST/ASTImporter.cpp:1029 + + RecordDecl *ToRD = cast(Importer.Import(cast(FromDC))); + Can't we just import the `FromRD`, why we need that cast at the end? `auto *ToRD = cast(Importer.Import(FromRD)));` Comment at: lib/AST/ASTImporter.cpp:1032 + for (auto *FromField : FromRD->fields()) { +Decl *ToField = Importer.GetAlreadyImportedOrNull(FromField); +assert(ToRD == ToField->getDeclContext() && ToRD->containsDecl(ToField)); I think `ToField` can be a nullptr, and if so, then `ToField->getDeclContext()` is UB. Same could happen at line 1040. Perhaps we should have and explicit check about the nullptr value: `if (!ToField) continue;` Comment at: unittests/AST/ASTImporterTest.cpp:1022 +AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector, Order) { + size_t Index = 0; The `hasFieldOrder` matcher is already existing. Comment at: unittests/AST/ASTImporterTest.cpp:1034 + +TEST(ImportDecl, ImportFieldOrder) { + MatchVerifier Verifier; We already have this test, we just have to enable it. `DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder` Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong updated this revision to Diff 147274. martong added a comment. Addressing review comments. Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1797,5 +1797,38 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +struct DeclContextTest : ASTImporterTestBase {}; + +TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { + Decl *TU = getTuDecl( + R"( + namespace NS { + + template + struct S {}; + template struct S; + + inline namespace INS { +template +struct S {}; +template struct S; + } + + } + )", Lang_CXX11, "input0.cc"); + auto *NS = FirstDeclMatcher().match( + TU, namespaceDecl()); + auto *Spec = FirstDeclMatcher().match( + TU, classTemplateSpecializationDecl()); + ASSERT_TRUE(NS->containsDecl(Spec)); + + NS->removeDecl(Spec); + EXPECT_FALSE(NS->containsDecl(Spec)); +} + +INSTANTIATE_TEST_CASE_P( +ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); + } // end namespace ast_matchers } // end namespace clang Index: lib/AST/DeclBase.cpp === --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,32 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +/// shouldBeHidden - Determine whether a declaration which was declared +/// within its semantic context should be invisible to qualified name lookup. +static bool shouldBeHidden(NamedDecl *D) { + // Skip unnamed declarations. + if (!D->getDeclName()) +return true; + + // Skip entities that can't be found by name lookup into a particular + // context. + if ((D->getIdentifierNamespace() == 0 && !isa(D)) || + D->isTemplateParameter()) +return true; + + // Skip template specializations. + // FIXME: This feels like a hack. Should DeclarationName support + // template-ids, or is there a better way to keep specializations + // from being visible? + if (isa(D)) +return true; + if (auto *FD = dyn_cast(D)) +if (FD->isFunctionTemplateSpecialization()) + return true; + + return false; +} + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,16 +1398,22 @@ } } } - + // Mark that D is no longer in the decl chain. D->NextInContextAndBits.setPointer(nullptr); // Remove D from the lookup table if necessary. if (isa(D)) { auto *ND = cast(D); +// Do not try to remove the declaration if that is invisible to qualified +// lookup. E.g. template specializations are skipped. +if (shouldBeHidden(ND)) + return; + // Remove only decls that have a name -if (!ND->getDeclName()) return; +if (!ND->getDeclName()) + return; auto *DC = D->getDeclContext(); do { @@ -1438,32 +1470,6 @@ makeDeclVisibleInContextWithFlags(ND, true, true); } -/// shouldBeHidden - Determine whether a declaration which was declared -/// within its semantic context should be invisible to qualified name lookup. -static bool shouldBeHidden(NamedDecl *D) { - // Skip unnamed declarations. - if (!D->getDeclName()) -return true; - - // Skip entities that can't be found by name lookup into a particular - // context. - if ((D->getIdentifierNamespace() == 0 && !isa(D)) || - D->isTemplateParameter()) -return true; - - // Skip template specializations. - // FIXME: This feels like a hack. Should DeclarationName support - // template-ids, or is there a better way to keep specializations - // from being visible? - if (isa(D)) -return true; - if (auto *FD = dyn_cast(D)) -if (FD->isFunctionTemplateSpecialization()) - return true; - - return false; -} - /// buildLookup - Build the lookup data structure with all of the /// declarations in this DeclContext (and any other contexts linked /// to it or transparent contexts nested within it) and return it. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl
martong added a comment. In https://reviews.llvm.org/D46958#1101570, @a.sidorin wrote: > So, we fail to add injected name to a CXXRecordDecl that has a described > class template? Yes, we failed to import the implicit `CXXRecordDecl`. Here is how it looked before the fix: From: ClassTemplateDecl 0xecae28 line:3:14 declToImport |-TemplateTypeParmDecl 0xecacd8 col:26 typename depth 0 index 0 U `-CXXRecordDecl 0xecad90 line:3:14 struct declToImport definition |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param | |-MoveConstructor exists simple trivial needs_implicit | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param | |-MoveAssignment exists simple trivial needs_implicit | `-Destructor simple irrelevant trivial needs_implicit `-CXXRecordDecl 0xecb0a0 col:14 implicit struct declToImport To: ClassTemplateDecl 0xf97cb0 col:14 declToImport |-TemplateTypeParmDecl 0xf97c00 col:26 typename depth 0 index 0 U `-CXXRecordDecl 0xf97a48 col:14 struct declToImport definition `-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param |-MoveConstructor exists simple trivial needs_implicit |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param |-MoveAssignment exists simple trivial needs_implicit `-Destructor simple irrelevant trivial needs_implicit Thanks for the review. Repository: rC Clang https://reviews.llvm.org/D46958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl
This revision was automatically updated to reflect the committed changes. Closed by commit rL332588: [ASTImporter] Fix missing implict CXXRecordDecl (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46958 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -2110,7 +2110,7 @@ D2 = D2CXX; D2->setAccess(D->getAccess()); D2->setLexicalDeclContext(LexicalDC); - if (!DCXX->getDescribedClassTemplate()) + if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit()) LexicalDC->addDeclInternal(D2); Importer.Imported(D, D2); Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1348,7 +1348,7 @@ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}; } -TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) { +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -2110,7 +2110,7 @@ D2 = D2CXX; D2->setAccess(D->getAccess()); D2->setLexicalDeclContext(LexicalDC); - if (!DCXX->getDescribedClassTemplate()) + if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit()) LexicalDC->addDeclInternal(D2); Importer.Imported(D, D2); Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1348,7 +1348,7 @@ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}; } -TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) { +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong marked 14 inline comments as done. martong added a comment. > Do you plan to enable this functionality for AST import checking? Yes. We'd like to test the structural equivalency independently from ASTImporter, because in certain cases it may have faulty behavior. This can be very handy also when we further extend structural equivalency. Comment at: unittests/AST/StructuralEquivalenceTest.cpp:119 + *cast(get<1>(t))->spec_begin(); + ASSERT_TRUE(Spec0 != nullptr); + ASSERT_TRUE(Spec1 != nullptr); a.sidorin wrote: > Should we assert for `spec_begin() != spec_end()` instead or within these > assertions? I think that is not needed, because the previous `cast` in line 116 and 118 would assert if `spec_begin() == spec_end()` was true. Comment at: unittests/AST/StructuralEquivalenceTest.cpp:174 + )"; + auto t = makeNamedDecls( Code0, Code0, Lang_CXX); + a.sidorin wrote: > 1. It's better to use more meaningful names than `t`. DeclTuple? > 2. The space after `(` is redundant. Renamed `t` to `Decls`. Repository: rC Clang https://reviews.llvm.org/D46867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong updated this revision to Diff 147304. martong marked an inline comment as done. martong added a comment. - Address aleksei's comments Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt unittests/AST/Language.cpp unittests/AST/Language.h unittests/AST/MatchVerifier.h unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/StructuralEquivalenceTest.cpp === --- /dev/null +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -0,0 +1,208 @@ +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/AST/ASTStructuralEquivalence.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Tooling/Tooling.h" + +#include "Language.h" +#include "DeclMatcher.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace ast_matchers { + +struct StructuralEquivalenceTest : ::testing::Test { + std::unique_ptr AST0, AST1; + std::string Code0, Code1; // Buffers for SourceManager + + // Get a pair of Decl pointers to the synthetised declarations from the given + // code snipets. By default we search for the unique Decl with name 'foo' in + // both snippets. + std::tuple + makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1, + Language Lang, const char *const Identifier = "foo") { + +this->Code0 = SrcCode0; +this->Code1 = SrcCode1; +ArgVector Args = getBasicRunOptionsForLanguage(Lang); + +const char *const InputFileName = "input.cc"; + +AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName); +AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName); + +ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext(); + +auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * { + IdentifierInfo *SearchedII = &Ctx.Idents.get(Name); + assert(SearchedII && "Declaration with the identifier " + "should be specified in test!"); + DeclarationName SearchDeclName(SearchedII); + SmallVector FoundDecls; + Ctx.getTranslationUnitDecl()->localUncachedLookup(SearchDeclName, +FoundDecls); + + // We should find one Decl but one only. + assert(FoundDecls.size() == 1); + + return FoundDecls[0]; +}; + +NamedDecl *D0 = getDecl(Ctx0, Identifier); +NamedDecl *D1 = getDecl(Ctx1, Identifier); +assert(D0); +assert(D1); +return std::make_tuple(D0, D1); + } + + bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) { +llvm::DenseSet> NonEquivalentDecls; +StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(), + NonEquivalentDecls, false, false); +return Ctx.IsStructurallyEquivalent(D0, D1); + } + + bool testStructuralMatch(std::tuple t) { +using std::get; +return testStructuralMatch(get<0>(t), get<1>(t)); + } +}; + +using std::get; + +TEST_F(StructuralEquivalenceTest, Int) { + auto Decls = makeNamedDecls("int foo;", "int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedInt) { + auto Decls = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, Char) { + auto Decls = makeNamedDecls("char foo;", "char foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +// This test is disabled for now. +// FIXME Whether this is equivalent is dependendant on the target. +TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) { + auto Decls = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) { + auto Decls = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) { + auto Decls = makeNamedDecls("struct foo { int x; };", + "struct foo { signed int x; };", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) { + auto Decls = makeNamedDecls("struct foo { char x; };", + "struct foo { signed char x; };", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) { + auto Decls = makeNamedDecls( + "template struct foo; template<> struct foo{};", + "template struct foo; template<> struct foo{};", + Lang_CXX); + ClassTemplateSpecializationDecl *Spec0 = + *cast(get<0>(Decls))->spec_begin(); + ClassTemplateSpecializationDecl *Spec1 = + *cast(get<1>(Decls))->spec_begin(); + ASSERT_TRUE(Spec0 != nullptr); + ASSERT_TRUE(Spec1 !
[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext
martong closed this revision. martong added a comment. Closed by commit: https://reviews.llvm.org/rL332699 Repository: rC Clang https://reviews.llvm.org/D46835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
martong created this revision. martong added reviewers: a.sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently we do not import the implicit CXXRecordDecl of a ClassTemplateSpecializationDecl. This patch fixes it. Repository: rC Clang https://reviews.llvm.org/D47057 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1367,7 +1367,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,7 +1959,7 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1367,7 +1367,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,7 +1959,7 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. ClassTemplateSpecialization is put in the wrong DeclContex if implicitly instantiated. This patch fixes it. Repository: rC Clang https://reviews.llvm.org/D47058 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1246,7 +1246,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4298,9 +4298,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->getTemplateSpecializationKind() != TSK_ImplicitInstantiation) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1246,7 +1246,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4298,9 +4298,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->getTemplateSpecializationKind() != TSK_ImplicitInstantiation) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47069: [ASTImporter] Enable disabled but passing test
martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. There is a test which passes since https://reviews.llvm.org/D32947, but it was forgotten to be enabled. This patch enables that disabled test. Repository: rC Clang https://reviews.llvm.org/D47069 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1399,7 +1399,7 @@ EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace()); } -TEST_P(ASTImporterTestBase, DISABLED_IDNSOfNonmemberOperator) { +TEST_P(ASTImporterTestBase, IDNSOfNonmemberOperator) { Decl *FromTU = getTuDecl( R"( struct X {}; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1399,7 +1399,7 @@ EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace()); } -TEST_P(ASTImporterTestBase, DISABLED_IDNSOfNonmemberOperator) { +TEST_P(ASTImporterTestBase, IDNSOfNonmemberOperator) { Decl *FromTU = getTuDecl( R"( struct X {}; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:6776 + // been invalidated to avoid repeatedly calling this. + ToContext.invalidateParents(); + Can an `Expr` has a parent too? If yes, why not invalidate the parents in `Import(Expr*)` ? I am also a bit concerned to call the invalidation during the import of all `Stmt` (and possible during the import of `Expr`s too). Isn't it enough to invalidate only during `Import(Decl*)`? https://reviews.llvm.org/D46940 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Ok, thanks for the explanation. Now it looks good to me. https://reviews.llvm.org/D46940 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47069: [ASTImporter] Enable disabled but passing test
This revision was automatically updated to reflect the committed changes. Closed by commit rC332728: [ASTImporter] Enable disabled but passing test (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47069?vs=147506&id=147524#toc Repository: rC Clang https://reviews.llvm.org/D47069 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1399,7 +1399,7 @@ EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace()); } -TEST_P(ASTImporterTestBase, DISABLED_IDNSOfNonmemberOperator) { +TEST_P(ASTImporterTestBase, IDNSOfNonmemberOperator) { Decl *FromTU = getTuDecl( R"( struct X {}; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1399,7 +1399,7 @@ EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace()); } -TEST_P(ASTImporterTestBase, DISABLED_IDNSOfNonmemberOperator) { +TEST_P(ASTImporterTestBase, IDNSOfNonmemberOperator) { Decl *FromTU = getTuDecl( R"( struct X {}; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
martong updated this revision to Diff 147966. martong marked an inline comment as done. martong added a comment. Use isExplicitInstantiationOrSpecialization Repository: rC Clang https://reviews.llvm.org/D47058 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1246,7 +1246,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4298,9 +4298,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->isExplicitInstantiationOrSpecialization()) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1246,7 +1246,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4298,9 +4298,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->isExplicitInstantiationOrSpecialization()) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4305 +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->getTemplateSpecializationKind() != TSK_ImplicitInstantiation) { + LexicalDC->addDeclInternal(D2); a.sidorin wrote: > Can we write `if (D2->isExplicitInstantiationOrSpecialization())` instead? > How we should treat TSK_Undeclared case? Yes, this is a good catch! Changed it. Repository: rC Clang https://reviews.llvm.org/D47058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1962 TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition); a.sidorin wrote: > We are changing import if RecordDecl. Is it possible to add a test that > doesn't require templates? > I tried and found that the implicit CXXRecordDecl of > ClassTemplateSpecializationDecl is its redeclaration. That's not true for > normal CXXRecordDecls, as I see, so this deserves a comment. Yes, I've added a new test which exercises only a RecordDecl. Also, added a comment about the specialization re-declaration. Repository: rC Clang https://reviews.llvm.org/D47057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
martong updated this revision to Diff 147994. martong marked an inline comment as done. martong added a comment. - Add new test case for simple CXXRecordDecl - Add comment about ClassTemplateSpecializationDecl implicit CXXRecordDecl Repository: rC Clang https://reviews.llvm.org/D47057 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1352,6 +1352,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1367,7 +1383,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,19 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() /*In contrast to a normal CXXRecordDecl, the implicit + CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + The definition of the implicit CXXRecordDecl in this case is the + ClassTemplateSpecializationDecl itself. Thus, we start with an extra + condition in order to be able to import the implict Decl.*/ + && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1352,6 +1352,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1367,7 +1383,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,19 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() /*In contrast to a normal CXXRecordDecl, the implicit + CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + The definition of the implicit CXXRecordDecl in this case is the + ClassTemplateSpecializationDecl itself. Thus, we start with an extra + condition in order to be able to import the implict Decl.*/ + && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem
martong updated this revision to Diff 148013. martong marked 5 inline comments as done. martong added a comment. - Addressing Aleksei's comments Repository: rC Clang https://reviews.llvm.org/D46950 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index: unittests/AST/DeclMatcher.h === --- unittests/AST/DeclMatcher.h +++ unittests/AST/DeclMatcher.h @@ -44,15 +44,23 @@ using FirstDeclMatcher = DeclMatcher; template -class DeclCounter : public MatchFinder::MatchCallback { +class DeclCounterWithPredicate : public MatchFinder::MatchCallback { + using UnaryPredicate = std::function; + UnaryPredicate Predicate; unsigned Count = 0; void run(const MatchFinder::MatchResult &Result) override { - if(Result.Nodes.getNodeAs("")) { +if (auto N = Result.Nodes.getNodeAs("")) { + if (Predicate(N)) ++Count; - } +} } + public: - // Returns the number of matched nodes under the tree rooted in `D`. + DeclCounterWithPredicate() + : Predicate([](const NodeType *) { return true; }) {} + DeclCounterWithPredicate(UnaryPredicate P) : Predicate(P) {} + // Returns the number of matched nodes which satisfy the predicate under the + // tree rooted in `D`. template unsigned match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; @@ -62,6 +70,9 @@ } }; +template +using DeclCounter = DeclCounterWithPredicate; + } // end namespace ast_matchers } // end namespace clang Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -281,8 +281,9 @@ return std::make_tuple(*FoundDecls.begin(), Imported); } - // Creates a TU decl for the given source code. - // May be called several times in a given test. + // Creates a TU decl for the given source code which can be used as a From + // context. May be called several times in a given test (with different file + // name). TranslationUnitDecl *getTuDecl(StringRef SrcCode, Language Lang, StringRef FileName = "input.cc") { assert( @@ -297,6 +298,16 @@ return Tu.TUDecl; } + // Creates the To context with the given source code and returns the TU decl. + TranslationUnitDecl *getToTuDecl(StringRef ToSrcCode, Language ToLang) { +ArgVector ToArgs = getArgVectorForLanguage(ToLang); +ToCode = ToSrcCode; +assert(!ToAST); +ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName); + +return ToAST->getASTContext().getTranslationUnitDecl(); + } + // Import the given Decl into the ToCtx. // May be called several times in a given test. // The different instances of the param From may have different ASTContext. @@ -1464,6 +1475,121 @@ } } +TEST_P(ASTImporterTestBase, + ImportDefinitionOfClassTemplateIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + template + struct B { +void f(); + }; + + template + struct B; + )", + Lang_CXX); + ASSERT_EQ(1u, DeclCounterWithPredicate( +[](const ClassTemplateDecl *T) { + return T->isThisDeclarationADefinition(); +}) +.match(ToTU, classTemplateDecl())); + + Decl *FromTU = getTuDecl( + R"( + template + struct B { +void f(); + }; + )", + Lang_CXX, "input1.cc"); + ClassTemplateDecl *FromD = FirstDeclMatcher().match( + FromTU, classTemplateDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + + // We should have only one definition. + EXPECT_EQ(1u, DeclCounterWithPredicate( +[](const ClassTemplateDecl *T) { + return T->isThisDeclarationADefinition(); +}) +.match(ToTU, classTemplateDecl())); +} + +TEST_P(ASTImporterTestBase, + ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) { + Decl *ToTU = getToTuDecl( + R"( + struct B { +void f(); + }; + + struct B; + )", + Lang_CXX); + ASSERT_EQ(2u, DeclCounter().match( +ToTU, cxxRecordDecl(hasParent(translationUnitDecl(); + + Decl *FromTU = getTuDecl( + R"( + struct B { +void f(); + }; + )", + Lang_CXX, "input1.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + + EXPECT_EQ(2u, DeclCounter().match( +ToTU, cxxRecordDecl(hasParent(translationUnitDecl(); +} + +TEST_P( +ASTImporterTestBase, +ImportDefinitionOfClassTemplateSpecIfThereIsAnExistingFwdDeclAndDefinition) +{ + Decl *ToTU = getToTuDecl( + R"( + template + struct B; + + template <> + struct B {}; +
[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem
This revision was automatically updated to reflect the committed changes. Closed by commit rC333082: Fix duplicate class template definitions problem (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D46950?vs=148013&id=148204#toc Repository: rC Clang https://reviews.llvm.org/D46950 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -4070,6 +4070,17 @@ TemplateParams); } +// Returns the definition for a (forward) declaration of a ClassTemplateDecl, if +// it has any definition in the redecl chain. +static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) { + CXXRecordDecl *ToTemplatedDef = D->getTemplatedDecl()->getDefinition(); + if (!ToTemplatedDef) +return nullptr; + ClassTemplateDecl *TemplateWithDef = + ToTemplatedDef->getDescribedClassTemplate(); + return TemplateWithDef; +} + Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { // If this record has a definition in the translation unit we're coming from, // but this particular declaration is not that definition, import the @@ -4084,7 +4095,7 @@ return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this class template. DeclContext *DC, *LexicalDC; DeclarationName Name; @@ -4103,34 +4114,39 @@ for (auto *FoundDecl : FoundDecls) { if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary)) continue; - + Decl *Found = FoundDecl; if (auto *FoundTemplate = dyn_cast(Found)) { -if (IsStructuralMatch(D, FoundTemplate)) { - // The class templates structurally match; call it the same template. - // We found a forward declaration but the class to be imported has a - // definition. - // FIXME Add this forward declaration to the redeclaration chain. - if (D->isThisDeclarationADefinition() && - !FoundTemplate->isThisDeclarationADefinition()) +// The class to be imported is a definition. +if (D->isThisDeclarationADefinition()) { + // Lookup will find the fwd decl only if that is more recent than the + // definition. So, try to get the definition if that is available in + // the redecl chain. + ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate); + if (!TemplateWithDef) continue; + FoundTemplate = TemplateWithDef; // Continue with the definition. +} - Importer.Imported(D->getTemplatedDecl(), +if (IsStructuralMatch(D, FoundTemplate)) { + // The class templates structurally match; call it the same template. + + Importer.Imported(D->getTemplatedDecl(), FoundTemplate->getTemplatedDecl()); return Importer.Imported(D, FoundTemplate); -} +} } - + ConflictingDecls.push_back(FoundDecl); } - + if (!ConflictingDecls.empty()) { Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary, - ConflictingDecls.data(), + ConflictingDecls.data(), ConflictingDecls.size()); } - + if (!Name) return nullptr; } Index: unittests/AST/DeclMatcher.h === --- unittests/AST/DeclMatcher.h +++ unittests/AST/DeclMatcher.h @@ -44,15 +44,23 @@ using FirstDeclMatcher = DeclMatcher; template -class DeclCounter : public MatchFinder::MatchCallback { +class DeclCounterWithPredicate : public MatchFinder::MatchCallback { + using UnaryPredicate = std::function; + UnaryPredicate Predicate; unsigned Count = 0; void run(const MatchFinder::MatchResult &Result) override { - if(Result.Nodes.getNodeAs("")) { +if (auto N = Result.Nodes.getNodeAs("")) { + if (Predicate(N)) ++Count; - } +} } + public: - // Returns the number of matched nodes under the tree rooted in `D`. + DeclCounterWithPredicate() + : Predicate([](const NodeType *) { return true; }) {} + DeclCounterWithPredicate(UnaryPredicate P) : Predicate(P) {} + // Returns the number of matched nodes which satisfy the predicate under the + // tree rooted in `D`. template unsigned match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; @@ -62,6 +70,9 @@ } }; +template +using DeclCounter = DeclCounterWithPredicate; + } // end namespace ast_matchers } // end namespace clang Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/AS
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
martong updated this revision to Diff 148209. martong added a comment. Remove multiline comment and reorder parts of the condition Repository: rC Clang https://reviews.llvm.org/D47057 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
This revision was automatically updated to reflect the committed changes. Closed by commit rL333086: [ASTImporter] Fix missing implict CXXRecordDecl in… (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47057 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl
This revision was automatically updated to reflect the committed changes. Closed by commit rC333086: [ASTImporter] Fix missing implict CXXRecordDecl in… (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47057?vs=148209&id=148210#toc Repository: rL LLVM https://reviews.llvm.org/D47057 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1363,6 +1363,22 @@ Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( + struct declToImport { + }; + )", + Lang_CXX, "", Lang_CXX); + + MatchVerifier Verifier; + // Match the implicit Decl. + auto Matcher = cxxRecordDecl(has(cxxRecordDecl())); + ASSERT_TRUE(Verifier.match(From, Matcher)); + EXPECT_TRUE(Verifier.match(To, Matcher)); +} + +TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"( template struct declToImport { }; @@ -1378,7 +1394,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { +ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) { Decl *From, *To; std::tie(From, To) = getImportedDecl( R"( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1959,14 +1959,20 @@ // but this particular declaration is not that definition, import the // definition and map to that. TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (Definition && Definition != D && + // In contrast to a normal CXXRecordDecl, the implicit + // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration. + // The definition of the implicit CXXRecordDecl in this case is the + // ClassTemplateSpecializationDecl itself. Thus, we start with an extra + // condition in order to be able to import the implict Decl. + !D->isImplicit()) { Decl *ImportedDef = Importer.Import(Definition); if (!ImportedDef) return nullptr; return Importer.Imported(D, ImportedDef); } - + // Import the major distinguishing characteristics of this record. DeclContext *DC, *LexicalDC; DeclarationName Name; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong updated this revision to Diff 148352. martong added a comment. Moved `using std::get` up, before `testStructuralMatch`. Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt unittests/AST/Language.cpp unittests/AST/Language.h unittests/AST/MatchVerifier.h unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/StructuralEquivalenceTest.cpp === --- /dev/null +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -0,0 +1,207 @@ +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/AST/ASTStructuralEquivalence.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Tooling/Tooling.h" + +#include "Language.h" +#include "DeclMatcher.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace ast_matchers { + +using std::get; + +struct StructuralEquivalenceTest : ::testing::Test { + std::unique_ptr AST0, AST1; + std::string Code0, Code1; // Buffers for SourceManager + + // Get a pair of Decl pointers to the synthetised declarations from the given + // code snipets. By default we search for the unique Decl with name 'foo' in + // both snippets. + std::tuple + makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1, + Language Lang, const char *const Identifier = "foo") { + +this->Code0 = SrcCode0; +this->Code1 = SrcCode1; +ArgVector Args = getBasicRunOptionsForLanguage(Lang); + +const char *const InputFileName = "input.cc"; + +AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName); +AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName); + +ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext(); + +auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * { + IdentifierInfo *SearchedII = &Ctx.Idents.get(Name); + assert(SearchedII && "Declaration with the identifier " + "should be specified in test!"); + DeclarationName SearchDeclName(SearchedII); + SmallVector FoundDecls; + Ctx.getTranslationUnitDecl()->localUncachedLookup(SearchDeclName, +FoundDecls); + + // We should find one Decl but one only. + assert(FoundDecls.size() == 1); + + return FoundDecls[0]; +}; + +NamedDecl *D0 = getDecl(Ctx0, Identifier); +NamedDecl *D1 = getDecl(Ctx1, Identifier); +assert(D0); +assert(D1); +return std::make_tuple(D0, D1); + } + + bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) { +llvm::DenseSet> NonEquivalentDecls; +StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(), + NonEquivalentDecls, false, false); +return Ctx.IsStructurallyEquivalent(D0, D1); + } + + bool testStructuralMatch(std::tuple t) { +return testStructuralMatch(get<0>(t), get<1>(t)); + } +}; + +TEST_F(StructuralEquivalenceTest, Int) { + auto Decls = makeNamedDecls("int foo;", "int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedInt) { + auto Decls = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, Char) { + auto Decls = makeNamedDecls("char foo;", "char foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +// This test is disabled for now. +// FIXME Whether this is equivalent is dependendant on the target. +TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) { + auto Decls = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) { + auto Decls = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) { + auto Decls = makeNamedDecls("struct foo { int x; };", + "struct foo { signed int x; };", Lang_CXX); + EXPECT_TRUE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) { + auto Decls = makeNamedDecls("struct foo { char x; };", + "struct foo { signed char x; };", Lang_CXX); + EXPECT_FALSE(testStructuralMatch(Decls)); +} + +TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) { + auto Decls = makeNamedDecls( + "template struct foo; template<> struct foo{};", + "template struct foo; template<> struct foo{};", + Lang_CXX); + ClassTemplateSpecializationDecl *Spec0 = + *cast(get<0>(Decls))->spec_begin(); + ClassTemplateSpecializationDecl *Spec1 = + *cast(get<1>(Decls))->spec_begin(); + ASSERT_TRUE(Spec0 != nullptr); + ASSERT_TRUE(Spec1 != nullptr); + EXPECT_TRUE(testStru
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
This revision was automatically updated to reflect the committed changes. Closed by commit rC333166: [ASTImporter] Add unit tests for structural equivalence (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D46867?vs=148352&id=148353#toc Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt unittests/AST/Language.cpp unittests/AST/Language.h unittests/AST/MatchVerifier.h unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -19,6 +19,7 @@ #include "clang/Tooling/Tooling.h" #include "DeclMatcher.h" +#include "Language.h" #include "gtest/gtest.h" #include "llvm/ADT/StringMap.h" @@ -29,50 +30,6 @@ using internal::BindableMatcher; using llvm::StringMap; -typedef std::vector ArgVector; -typedef std::vector RunOptions; - -static bool isCXX(Language Lang) { - return Lang == Lang_CXX || Lang == Lang_CXX11; -} - -static ArgVector getBasicRunOptionsForLanguage(Language Lang) { - ArgVector BasicArgs; - // Test with basic arguments. - switch (Lang) { - case Lang_C: -BasicArgs = {"-x", "c", "-std=c99"}; -break; - case Lang_C89: -BasicArgs = {"-x", "c", "-std=c89"}; -break; - case Lang_CXX: -BasicArgs = {"-std=c++98", "-frtti"}; -break; - case Lang_CXX11: -BasicArgs = {"-std=c++11", "-frtti"}; -break; - case Lang_OpenCL: - case Lang_OBJCXX: -llvm_unreachable("Not implemented yet!"); - } - return BasicArgs; -} - -static RunOptions getRunOptionsForLanguage(Language Lang) { - ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang); - - // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC - // default behaviour. - if (isCXX(Lang)) { -ArgVector ArgsForDelayedTemplateParse = BasicArgs; -ArgsForDelayedTemplateParse.emplace_back("-fdelayed-template-parsing"); -return {BasicArgs, ArgsForDelayedTemplateParse}; - } - - return {BasicArgs}; -} - // Creates a virtual file and assigns that to the context of given AST. If the // file already exists then the file will not be created again as a duplicate. static void Index: unittests/AST/MatchVerifier.h === --- unittests/AST/MatchVerifier.h +++ unittests/AST/MatchVerifier.h @@ -23,20 +23,12 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/Tooling.h" +#include "Language.h" #include "gtest/gtest.h" namespace clang { namespace ast_matchers { -enum Language { -Lang_C, -Lang_C89, -Lang_CXX, -Lang_CXX11, -Lang_OpenCL, -Lang_OBJCXX -}; - /// \brief Base class for verifying some property of nodes found by a matcher. template class MatchVerifier : public MatchFinder::MatchCallback { @@ -113,6 +105,10 @@ Args.push_back("-std=c++11"); FileName = "input.cc"; break; + case Lang_CXX14: +Args.push_back("-std=c++14"); +FileName = "input.cc"; +break; case Lang_OpenCL: FileName = "input.cl"; break; Index: unittests/AST/CMakeLists.txt === --- unittests/AST/CMakeLists.txt +++ unittests/AST/CMakeLists.txt @@ -15,9 +15,11 @@ DeclTest.cpp EvaluateAsRValueTest.cpp ExternalASTSourceTest.cpp + Language.cpp NamedDeclPrinterTest.cpp SourceLocationTest.cpp StmtPrinterTest.cpp + StructuralEquivalenceTest.cpp ) target_link_libraries(ASTTests Index: unittests/AST/StructuralEquivalenceTest.cpp === --- unittests/AST/StructuralEquivalenceTest.cpp +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -0,0 +1,207 @@ +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/AST/ASTStructuralEquivalence.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/Tooling/Tooling.h" + +#include "Language.h" +#include "DeclMatcher.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace ast_matchers { + +using std::get; + +struct StructuralEquivalenceTest : ::testing::Test { + std::unique_ptr AST0, AST1; + std::string Code0, Code1; // Buffers for SourceManager + + // Get a pair of Decl pointers to the synthetised declarations from the given + // code snipets. By default we search for the unique Decl with name 'foo' in + // both snippets. + std::tuple + makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1, + Language Lang, const char *const Identifier = "foo") { + +this->Code0 = SrcCode0; +this->Code1 = SrcCode1; +ArgVector Args = getBasicRunOptionsForLanguage(Lang); + +const char *const InputFileName = "input.cc"; + +AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args,
[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl
martong added a comment. Looks good to me, but let's wait for Aleksei's approval and comments. Repository: rC Clang https://reviews.llvm.org/D47313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
martong added a comment. > Could you add a test for TSK_Undeclared as well? TLDR: No, I cannot. `TSK_Undeclared` is used to indicate that a template specialization was formed from a template-id but has not yet been declared, defined, or instantiated. Consequently, `ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl` and `VarTemplateSpecializationDecl::VarTemplateSpecializationDecl` initializes its member `SpecializationKind` to the value `TSK_Undeclared`. `SpecializationKind` is getting set during parsing to a meaningful kind, it is never left as `TSK_Undeclared`. So, the only way I could write a test is to manually change the `SpecializationKind` in the "From" context, but I consider that as not a meaningful test, since we cannot create such `Decl` by the parser. Repository: rC Clang https://reviews.llvm.org/D47058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
This revision was automatically updated to reflect the committed changes. Closed by commit rL333269: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47058 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -4320,9 +4320,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->isExplicitInstantiationOrSpecialization()) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1214,7 +1214,7 @@ TEST_P( ASTImporterTestBase, - DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -4320,9 +4320,13 @@ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind()); -// Add the specialization to this context. +// Set the context of this specialization/instantiation. D2->setLexicalDeclContext(LexicalDC); -LexicalDC->addDeclInternal(D2); + +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->isExplicitInstantiationOrSpecialization()) { + LexicalDC->addDeclInternal(D2); +} } Importer.Imported(D, D2); if (D->isCompleteDefinition() && ImportDefinition(D, D2)) Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -1214,7 +1214,7 @@ TEST_P( ASTImporterTestBase, -DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { +TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) { Decl *From, *To; std::tie(From, To) = getImportedDecl( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase
martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. In order to avoid build failures on MS, we use -fms-compatibility too in the tests which use the TestBase. Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1563,10 +1563,6 @@ .match(ToTU, classTemplateSpecializationDecl())); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, ASTImporterTestBase, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); - struct ImportFunctions : ASTImporterTestBase {}; TEST_P(ImportFunctions, @@ -1791,10 +1787,6 @@ EXPECT_TRUE(To->isVirtual()); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, ImportFunctions, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); - AST_MATCHER_P(TagDecl, hasTypedefForAnonDecl, Matcher, InnerMatcher) { if (auto *Typedef = Node.getTypedefNameForAnonDecl()) @@ -1925,9 +1917,20 @@ EXPECT_FALSE(NS->containsDecl(Spec)); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, DeclContextTest, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector()), ); + +auto DefaultTestValuesForRunOptions = ::testing::Values( +ArgVector(), +ArgVector{"-fdelayed-template-parsing"}, +ArgVector{"-fms-compatibility"}, +ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"}); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterTestBase, +DefaultTestValuesForRunOptions, ); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions, +DefaultTestValuesForRunOptions, ); } // end namespace ast_matchers } // end namespace clang Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1563,10 +1563,6 @@ .match(ToTU, classTemplateSpecializationDecl())); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, ASTImporterTestBase, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); - struct ImportFunctions : ASTImporterTestBase {}; TEST_P(ImportFunctions, @@ -1791,10 +1787,6 @@ EXPECT_TRUE(To->isVirtual()); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, ImportFunctions, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); - AST_MATCHER_P(TagDecl, hasTypedefForAnonDecl, Matcher, InnerMatcher) { if (auto *Typedef = Node.getTypedefNameForAnonDecl()) @@ -1925,9 +1917,20 @@ EXPECT_FALSE(NS->containsDecl(Spec)); } -INSTANTIATE_TEST_CASE_P( -ParameterizedTests, DeclContextTest, -::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, +::testing::Values(ArgVector()), ); + +auto DefaultTestValuesForRunOptions = ::testing::Values( +ArgVector(), +ArgVector{"-fdelayed-template-parsing"}, +ArgVector{"-fms-compatibility"}, +ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"}); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterTestBase, +DefaultTestValuesForRunOptions, ); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions, +DefaultTestValuesForRunOptions, ); } // end namespace ast_matchers } // end namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem
martong added a comment. @a.sidorin Yes, that is a very good idea. Just created a new patch which adds that switch for the tests which use the TestBase. https://reviews.llvm.org/D47367 Repository: rC Clang https://reviews.llvm.org/D46950 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase
martong added a comment. This patch does not target `testImport` because I am not sure how to handle the options there: auto RunOptsFrom = getRunOptionsForLanguage(FromLang); auto RunOptsTo = getRunOptionsForLanguage(ToLang); for (const auto &FromArgs : RunOptsFrom) for (const auto &ToArgs : RunOptsTo) EXPECT_TRUE(testImport(FromCode, FromArgs, ToCode, ToArgs, Verifier, AMatcher)); I think, it is overkill to test all possible combinations of the options, we should come up with something better here. Perhaps we could exploit parameterized tests here as well. @a.sidorin, @xazax.hun What is your opinion? Repository: rC Clang https://reviews.llvm.org/D47367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait for someone else's review too. @a.sidorin We discovered this error during the CTU analysis of google/protobuf and we could reduce the erroneous C file with c-reduce to the minimal example presented in the test file. Repository: rC Clang https://reviews.llvm.org/D47450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.
martong added a reviewer: r.stahl. martong added a comment. Adding Rafael too as a reviewer, because he has been working also on the ASTImporter recently. Repository: rC Clang https://reviews.llvm.org/D47450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
martong added reviewers: akyrtzi, ddunbar. martong added a comment. Adding Argyrios and Daniel as a reviewer for ASTUnit related changes. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait the review of those people who have history in `ASTUnit.cpp`. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase
martong added a comment. > I agree with that. I think we need to test just import pairs > {/*From*/no_option, /*To*/no_option}, {option_1, option1}, {option_2, > option_2}, ...{option_n, option_n}. This patch does exactly that with the parameterized tests. Each elements of the `DefaultTestValuesForRunOptions` will be used both for the "To" and for the "From" context. So we will have each TEST_P generating 4 cases: [FROM, TO] {no_option, no_option} {"-fdelayed-template-parsing", "-fdelayed-template-parsing"} {"-fms-compatibility", "-fms-compatibility"} {"-fdelayed-template-parsing -fms-compatibility", "-fdelayed-template-parsing -fms-compatibility"} Repository: rC Clang https://reviews.llvm.org/D47367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl
martong added a comment. Balazs, I'll commit it for you in an hour. Repository: rC Clang https://reviews.llvm.org/D47313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl
This revision was automatically updated to reflect the committed changes. Closed by commit rC333522: [ASTImporter] Corrected lookup at import of templated record decl (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47313?vs=148361&id=149065#toc Repository: rC Clang https://reviews.llvm.org/D47313 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2015,7 +2015,14 @@ if (const auto *Tag = Typedef->getUnderlyingType()->getAs()) Found = Tag->getDecl(); } - + + if (D->getDescribedTemplate()) { +if (auto *Template = dyn_cast(Found)) + Found = Template->getTemplatedDecl(); +else + continue; + } + if (auto *FoundRecord = dyn_cast(Found)) { if (!SearchName) { // If both unnamed structs/unions are in a record context, make sure Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1107,6 +1107,50 @@ has(fieldDecl(hasType(dependentSizedArrayType(; } +TEST_P(ASTImporterTestBase, ImportOfTemplatedDeclOfClassTemplateDecl) { + Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX); + auto From = + FirstDeclMatcher().match(FromTU, classTemplateDecl()); + ASSERT_TRUE(From); + auto To = cast(Import(From, Lang_CXX)); + ASSERT_TRUE(To); + Decl *ToTemplated = To->getTemplatedDecl(); + Decl *ToTemplated1 = Import(From->getTemplatedDecl(), Lang_CXX); + EXPECT_TRUE(ToTemplated1); + EXPECT_EQ(ToTemplated1, ToTemplated); +} + +TEST_P(ASTImporterTestBase, ImportCorrectTemplatedDecl) { + auto Code = +R"( +namespace x { + template struct S1{}; + template struct S2{}; + template struct S3{}; +} +)"; + Decl *FromTU = getTuDecl(Code, Lang_CXX); + auto FromNs = + FirstDeclMatcher().match(FromTU, namespaceDecl()); + auto ToNs = cast(Import(FromNs, Lang_CXX)); + ASSERT_TRUE(ToNs); + auto From = + FirstDeclMatcher().match(FromTU, + classTemplateDecl( + hasName("S2"))); + auto To = + FirstDeclMatcher().match(ToNs, + classTemplateDecl( + hasName("S2"))); + ASSERT_TRUE(From); + ASSERT_TRUE(To); + auto ToTemplated = To->getTemplatedDecl(); + auto ToTemplated1 = + cast(Import(From->getTemplatedDecl(), Lang_CXX)); + EXPECT_TRUE(ToTemplated1); + ASSERT_EQ(ToTemplated1, ToTemplated); +} + TEST_P(ASTImporterTestBase, DISABLED_ImportFunctionWithBackReferringParameter) { Decl *From, *To; std::tie(From, To) = getImportedDecl( Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2015,7 +2015,14 @@ if (const auto *Tag = Typedef->getUnderlyingType()->getAs()) Found = Tag->getDecl(); } - + + if (D->getDescribedTemplate()) { +if (auto *Template = dyn_cast(Found)) + Found = Template->getTemplatedDecl(); +else + continue; + } + if (auto *FoundRecord = dyn_cast(Found)) { if (!SearchName) { // If both unnamed structs/unions are in a record context, make sure Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1107,6 +1107,50 @@ has(fieldDecl(hasType(dependentSizedArrayType(; } +TEST_P(ASTImporterTestBase, ImportOfTemplatedDeclOfClassTemplateDecl) { + Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX); + auto From = + FirstDeclMatcher().match(FromTU, classTemplateDecl()); + ASSERT_TRUE(From); + auto To = cast(Import(From, Lang_CXX)); + ASSERT_TRUE(To); + Decl *ToTemplated = To->getTemplatedDecl(); + Decl *ToTemplated1 = Import(From->getTemplatedDecl(), Lang_CXX); + EXPECT_TRUE(ToTemplated1); + EXPECT_EQ(ToTemplated1, ToTemplated); +} + +TEST_P(ASTImporterTestBase, ImportCorrectTemplatedDecl) { + auto Code = +R"( +namespace x { + template struct S1{}; + template struct S2{}; + template struct S3{}; +} +)"; + Decl *FromTU = getTuDecl(Code, Lang_CXX); + auto FromNs = + FirstDeclMatcher().match(FromTU, namespaceDecl()); + auto ToNs = cast(Import(FromNs, Lang_CXX)); + ASSERT_TRUE(ToNs); + auto From = + FirstDeclMatcher().match(FromTU, + classTemplateD
[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation
martong added inline comments. Comment at: lib/StaticAnalyzer/Core/IssueHash.cpp:39 + // primary template. + if (const FunctionDecl *InstantiatedFrom = + Target->getInstantiatedFromMemberFunction()) Could we use here FunctionDecl::getPrimaryTemplate() ? That seems more general, it handles both specializations and instantiations. Comment at: test/Analysis/bug_hash_test.cpp:61 -// CHECK: diagnostics +template +T f(T i) { We could add a few more test cases: - a function template in class template - specializations vs instantiations - the combination of the above two (?) Comment at: test/Analysis/bug_hash_test.cpp:1363 // CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path I am not sure if this is possible, but could we add unit test just for the `GetSignature` function? Instead of these huge plist files? I am thinking something like this: https://github.com/martong/friend-stats/blob/ed0c69ea3669c933204c799f59b85cd7b2507c34/ut/FriendFunctionsTest.cpp#L31 Repository: rL LLVM https://reviews.llvm.org/D38728 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation
martong added inline comments. Comment at: test/Analysis/bug_hash_test.cpp:105 +void g() { + TX x; + TX xl; As we discussed, the checking of the equality of the `IssueString` in case of `TX` and `TX` is implicit. And as such it is hard to see that it is really tested. Perhaps a comment here could indicate this implicit checking mechanism. https://reviews.llvm.org/D38728 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc
martong added a comment. Consider the use of a function pointer: void* malloc(int); int strlen(char*); auto fp = malloc; void bad_malloc(char *str) { char *c = (char *)fp(strlen(str + 1)); } I think, the checker will not match in this case. One might use allocation functions via a function pointer in case of more possible allocation strategies (e.g having a different strategy for a shared memory allocation). https://reviews.llvm.org/D39121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc
martong added a comment. We might get false positives in case of certain substring operations. Consider the case of copying a substring, pseudo code below: const char * s = "abcdefg"; int offset = my_find('d', s); // I want to copy "defg" char *new_subststring = (char*) malloc(strlen(s + offset)); strcpy(...); https://reviews.llvm.org/D39121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51533: [ASTImporter] Merge ExprBits
martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Some `Expr` classes set up default values for the `ExprBits` of `Stmt`. These default values are then overwritten by the parser sometimes. One example is `InitListExpr` which sets the value kind to be an rvalue in the ctor. However, this bit may change after the `InitListExpr` is created. There may be other expressions similar to `InitListExpr` in this sense, thus the safest solution is to copy the expression bits. The lack of copying `ExprBits` causes an assertion in the analyzer engine in a specific case: Since the value kind is not imported, the analyzer engine believes that the given InitListExpr is an rvalue, thus it creates a nonloc::CompoundVal instead of creating memory region (as in case of an lvalue reference). Repository: rC Clang https://reviews.llvm.org/D51533 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -3225,6 +3225,25 @@ unless(classTemplatePartialSpecializationDecl(); } +TEST_P(ASTImporterTestBase, InitListExprValueKindShouldBeImported) { + Decl *TU = getTuDecl( + R"( + const int &init(); + void foo() { const int &a{init()}; } + )", Lang_CXX11, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(TU, varDecl(hasName("a"))); + ASSERT_TRUE(FromD->getAnyInitializer()); + auto *InitExpr = FromD->getAnyInitializer(); + ASSERT_TRUE(InitExpr); + ASSERT_TRUE(InitExpr->isGLValue()); + + auto *ToD = Import(FromD, Lang_CXX11); + ASSERT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); + ASSERT_TRUE(ToInitExpr); + EXPECT_TRUE(ToInitExpr->isGLValue()); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -6849,9 +6849,9 @@ To->setSyntacticForm(ToSyntForm); } + // Copy InitListExprBitfields, which are not handled in the ctor of + // InitListExpr. To->sawArrayRangeDesignator(ILE->hadArrayRangeDesignator()); - To->setValueDependent(ILE->isValueDependent()); - To->setInstantiationDependent(ILE->isInstantiationDependent()); return To; } @@ -7196,6 +7196,19 @@ if (!ToS) return nullptr; + if (auto *ToE = dyn_cast(ToS)) { +auto *FromE = cast(FromS); +// Copy ExprBitfields, which may not be handled in Expr subclasses +// constructors. +ToE->setValueKind(FromE->getValueKind()); +ToE->setObjectKind(FromE->getObjectKind()); +ToE->setTypeDependent(FromE->isTypeDependent()); +ToE->setValueDependent(FromE->isValueDependent()); +ToE->setInstantiationDependent(FromE->isInstantiationDependent()); +ToE->setContainsUnexpandedParameterPack( +FromE->containsUnexpandedParameterPack()); + } + // Record the imported declaration. ImportedStmts[FromS] = ToS; return ToS; Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -3225,6 +3225,25 @@ unless(classTemplatePartialSpecializationDecl(); } +TEST_P(ASTImporterTestBase, InitListExprValueKindShouldBeImported) { + Decl *TU = getTuDecl( + R"( + const int &init(); + void foo() { const int &a{init()}; } + )", Lang_CXX11, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(TU, varDecl(hasName("a"))); + ASSERT_TRUE(FromD->getAnyInitializer()); + auto *InitExpr = FromD->getAnyInitializer(); + ASSERT_TRUE(InitExpr); + ASSERT_TRUE(InitExpr->isGLValue()); + + auto *ToD = Import(FromD, Lang_CXX11); + ASSERT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); + ASSERT_TRUE(ToInitExpr); + EXPECT_TRUE(ToInitExpr->isGLValue()); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -6849,9 +6849,9 @@ To->setSyntacticForm(ToSyntForm); } + // Copy InitListExprBitfields, which are not handled in the ctor of + // InitListExpr. To->sawArrayRangeDesignator(ILE->hadArrayRangeDesignator()); - To->setValueDependent(ILE->isValueDependent()); - To->setInstantiationDependent(ILE->isInstantiationDependent()); return To; } @@ -7196,6 +7196,19 @@ if (!ToS) return nullptr; + if (auto *ToE = dyn_cast(ToS)) { +auto *FromE = cast(FromS); +
[PATCH] D51533: [ASTImporter] Merge ExprBits
martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:3241 + auto *ToD = Import(FromD, Lang_CXX11); + ASSERT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); a_sidorin wrote: > EXPECT_TRUE (same below). Thanks, changed it. Repository: rL LLVM https://reviews.llvm.org/D51533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51533: [ASTImporter] Merge ExprBits
This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rL341316: [ASTImporter] Merge ExprBits (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51533?vs=163494&id=163707#toc Repository: rL LLVM https://reviews.llvm.org/D51533 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -6817,9 +6817,9 @@ To->setSyntacticForm(ToSyntForm); } + // Copy InitListExprBitfields, which are not handled in the ctor of + // InitListExpr. To->sawArrayRangeDesignator(ILE->hadArrayRangeDesignator()); - To->setValueDependent(ILE->isValueDependent()); - To->setInstantiationDependent(ILE->isInstantiationDependent()); return To; } @@ -7164,6 +7164,19 @@ if (!ToS) return nullptr; + if (auto *ToE = dyn_cast(ToS)) { +auto *FromE = cast(FromS); +// Copy ExprBitfields, which may not be handled in Expr subclasses +// constructors. +ToE->setValueKind(FromE->getValueKind()); +ToE->setObjectKind(FromE->getObjectKind()); +ToE->setTypeDependent(FromE->isTypeDependent()); +ToE->setValueDependent(FromE->isValueDependent()); +ToE->setInstantiationDependent(FromE->isInstantiationDependent()); +ToE->setContainsUnexpandedParameterPack( +FromE->containsUnexpandedParameterPack()); + } + // Record the imported declaration. ImportedStmts[FromS] = ToS; return ToS; Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -3225,6 +3225,25 @@ unless(classTemplatePartialSpecializationDecl(); } +TEST_P(ASTImporterTestBase, InitListExprValueKindShouldBeImported) { + Decl *TU = getTuDecl( + R"( + const int &init(); + void foo() { const int &a{init()}; } + )", Lang_CXX11, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(TU, varDecl(hasName("a"))); + ASSERT_TRUE(FromD->getAnyInitializer()); + auto *InitExpr = FromD->getAnyInitializer(); + ASSERT_TRUE(InitExpr); + ASSERT_TRUE(InitExpr->isGLValue()); + + auto *ToD = Import(FromD, Lang_CXX11); + EXPECT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); + EXPECT_TRUE(ToInitExpr); + EXPECT_TRUE(ToInitExpr->isGLValue()); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -6817,9 +6817,9 @@ To->setSyntacticForm(ToSyntForm); } + // Copy InitListExprBitfields, which are not handled in the ctor of + // InitListExpr. To->sawArrayRangeDesignator(ILE->hadArrayRangeDesignator()); - To->setValueDependent(ILE->isValueDependent()); - To->setInstantiationDependent(ILE->isInstantiationDependent()); return To; } @@ -7164,6 +7164,19 @@ if (!ToS) return nullptr; + if (auto *ToE = dyn_cast(ToS)) { +auto *FromE = cast(FromS); +// Copy ExprBitfields, which may not be handled in Expr subclasses +// constructors. +ToE->setValueKind(FromE->getValueKind()); +ToE->setObjectKind(FromE->getObjectKind()); +ToE->setTypeDependent(FromE->isTypeDependent()); +ToE->setValueDependent(FromE->isValueDependent()); +ToE->setInstantiationDependent(FromE->isInstantiationDependent()); +ToE->setContainsUnexpandedParameterPack( +FromE->containsUnexpandedParameterPack()); + } + // Record the imported declaration. ImportedStmts[FromS] = ToS; return ToS; Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -3225,6 +3225,25 @@ unless(classTemplatePartialSpecializationDecl(); } +TEST_P(ASTImporterTestBase, InitListExprValueKindShouldBeImported) { + Decl *TU = getTuDecl( + R"( + const int &init(); + void foo() { const int &a{init()}; } + )", Lang_CXX11, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(TU, varDecl(hasName("a"))); + ASSERT_TRUE(FromD->getAnyInitializer()); + auto *InitExpr = FromD->getAnyInitializer(); + ASSERT_TRUE(InitExpr); + ASSERT_TRUE(InitExpr->isGLValue()); + + auto *ToD = Import(FromD, Lang_CXX11); + EXPECT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); + EXPECT_TRUE(ToInitExpr); + EXPECT_TRUE(ToInitExpr->isGLValue()); +} + str
[PATCH] D51597: [ASTImporter] Fix import of VarDecl init
martong created this revision. martong added reviewers: a_sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: a.sidorin. The init expression of a VarDecl is overwritten in the "To" context if we import a VarDecl without an init expression (and with a definition). Please refer to the added tests, especially InitAndDefinitionAreInDifferentTUs. This patch fixes the malfunction by importing the whole Decl chain similarly as we did that in case of FunctionDecls. We handle the init expression similarly to a definition, alas only one init expression will be in the merged ast. Repository: rC Clang https://reviews.llvm.org/D51597 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1828,13 +1828,62 @@ { Decl *FromTU = getTuDecl("extern int x; int f() { return x; }", Lang_CXX, "input2.cc"); -auto *FromD = -FirstDeclMatcher().match(FromTU, functionDecl()); +auto *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); Import(FromD, Lang_CXX); } EXPECT_TRUE(Imported2->isUsed(false)); } +TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag2) { + auto Pattern = varDecl(hasName("x")); + VarDecl *ExistingD; + { +Decl *ToTU = getToTuDecl("int x = 1;", Lang_CXX); +ExistingD = FirstDeclMatcher().match(ToTU, Pattern); + } + EXPECT_FALSE(ExistingD->isUsed(false)); + { +Decl *FromTU = getTuDecl( +"int x = 1; int f() { return x; }", Lang_CXX, "input1.cc"); +auto *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +Import(FromD, Lang_CXX); + } + EXPECT_TRUE(ExistingD->isUsed(false)); +} + +TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag3) { + auto Pattern = varDecl(hasName("a")); + VarDecl *ExistingD; + { +Decl *ToTU = getToTuDecl( +R"( +struct A { + static const int a = 1; +}; +)", Lang_CXX); +ExistingD = FirstDeclMatcher().match(ToTU, Pattern); + } + EXPECT_FALSE(ExistingD->isUsed(false)); + { +Decl *FromTU = getTuDecl( +R"( +struct A { + static const int a = 1; +}; +const int *f() { return &A::a; } // requires storage, + // thus used flag will be set +)", Lang_CXX, "input1.cc"); +auto *FromFunD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +auto *FromD = FirstDeclMatcher().match(FromTU, Pattern); +ASSERT_TRUE(FromD->isUsed(false)); +Import(FromFunD, Lang_CXX); + } + EXPECT_TRUE(ExistingD->isUsed(false)); +} + TEST_P(ASTImporterTestBase, ReimportWithUsedFlag) { auto Pattern = varDecl(hasName("x")); @@ -3244,6 +3293,94 @@ EXPECT_TRUE(ToInitExpr->isGLValue()); } +struct ImportVariables : ASTImporterTestBase {}; + +TEST_P(ImportVariables, ImportOfOneDeclBringsInTheWholeChain) { + Decl *FromTU = getTuDecl( + R"( + struct A { +static const int a = 1 + 2; + }; + const int A::a; + )", Lang_CXX, "input1.cc"); + + auto *FromDWithInit = FirstDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with definition + ASSERT_NE(FromDWithInit, FromDWithDef); + ASSERT_EQ(FromDWithDef->getPreviousDecl() ,FromDWithInit); + + auto *ToD0 = cast(Import(FromDWithInit, Lang_CXX11)); + auto *ToD1 = cast(Import(FromDWithDef, Lang_CXX11)); + ASSERT_TRUE(ToD0); + ASSERT_TRUE(ToD1); + EXPECT_NE(ToD0, ToD1); + EXPECT_EQ(ToD1->getPreviousDecl() ,ToD0); +} + +TEST_P(ImportVariables, InitAndDefinitionAreInDifferentTUs) { + auto StructA = + R"( + struct A { +static const int a = 1 + 2; + }; + )"; + Decl *ToTU = getToTuDecl(StructA, Lang_CXX); + Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a;", Lang_CXX, + "input1.cc"); + + auto *FromDWithInit = FirstDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with definition + ASSERT_EQ(FromDWithInit, FromDWithDef->getPreviousDecl()); + ASSERT_TRUE(FromDWithInit->getInit()); + ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition()); + ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition()); + ASSERT_FALSE(FromDWithDef->getInit()); + + auto *ToD = FirstDeclMatcher().match( + ToTU, varDecl(hasName("a"))); // Decl with init + ASSERT_TRUE(ToD->getInit()); + ASSERT_FALSE(ToD->getDefinition()); + + auto *ImportedD = cast(Import(FromDWithDef, Lang_CXX11)); + EXPECT_TRUE(ImportedD->getAnyInitializer()); + EX
[PATCH] D51597: [ASTImporter] Fix import of VarDecl init
martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:3763 +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods, +DefaultTestValuesForRunOptions, ); This hunk has nothing to do with this change, but previously we forgot to instantiate these test cases :( Repository: rC Clang https://reviews.llvm.org/D51597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. Here is the `Expected` based patch: https://reviews.llvm.org/D51633 Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
martong added a comment. This patch is huge, but we change here almost all implementation functions of `ASTNodeImporter` to return with either `Error` or with `Expected`. We could not really come up with a cohesive but smaller patch because of the recursive nature of the importer. (We are open to ideas about how to cut this patch into smaller parts.) Most of the changes are trivial: we always have to check the return value of any import function and pass on the error to the caller if there was any. In my opinion one great simplification is the introduction of the auxiliary `importSeq` function. It imports each entity one-by-one after each other and in case of any error it returns with that error and does not continue with the subsequent import operations. Repository: rC Clang https://reviews.llvm.org/D51633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements
martong added a comment. From this little information I have hear are my thoughts: > match callExpr(allOf (callee(functionDecl(hasName("std::sort"))), > hasArgument(0, > hasDescendant(declRefExpr(to(fieldDecl(hasName("value_type")) I think this is a good direction, but keep in mind that `value_type` is a typedef, thus you should use the `typedefNameDecl` matcher instead of the `fieldDecl`. (Also if I understand correctly then this is good that this matcher does not match in case of the `intPointerArray` example, because the array does not have any member at all ...) Repository: rC Clang https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51597: [ASTImporter] Fix import of VarDecl init
martong marked 3 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1441 + To->setInit(ToInit); + if (From->isInitKnownICE()) { +EvaluatedStmt *Eval = To->ensureEvaluatedStmt(); a_sidorin wrote: > I see that this is only a code move but I realized that ASTReader and > ASTImporter handle this case differently. ASTReader code says: > > ``` > if (Val > 1) { // IsInitKnownICE = 1, IsInitNotICE = 2, IsInitICE = 3 > EvaluatedStmt *Eval = VD->ensureEvaluatedStmt(); > Eval->CheckedICE = true; > Eval->IsICE = Val == 3; > } > ``` > but ASTimporter sets these bits only if `isInitKnownICE()` is `true`. This > looks strange. The comment in ASTReader seems to be wrong and misleading. I checked the correspondent code in ASTWriter: ``` Record.push_back(!D->isInitKnownICE() ? 1 : (D->isInitICE() ? 3 : 2)); ``` Thus, the comment in ASTReader should be: ``` if (Val > 1) { // IsInitNOTKnownICE = 1, IsInitNotICE = 2, IsInitICE = 3 ``` So, `Val > 1` means that the original init expression written by the ASTWriter had the ICE-ness already determined. Thus the ASTImporter code seems correct to me. Repository: rC Clang https://reviews.llvm.org/D51597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51597: [ASTImporter] Fix import of VarDecl init
martong updated this revision to Diff 165039. martong marked an inline comment as done. martong added a comment. - Fix formatting and typo Repository: rC Clang https://reviews.llvm.org/D51597 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1828,13 +1828,62 @@ { Decl *FromTU = getTuDecl("extern int x; int f() { return x; }", Lang_CXX, "input2.cc"); -auto *FromD = -FirstDeclMatcher().match(FromTU, functionDecl()); +auto *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); Import(FromD, Lang_CXX); } EXPECT_TRUE(Imported2->isUsed(false)); } +TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag2) { + auto Pattern = varDecl(hasName("x")); + VarDecl *ExistingD; + { +Decl *ToTU = getToTuDecl("int x = 1;", Lang_CXX); +ExistingD = FirstDeclMatcher().match(ToTU, Pattern); + } + EXPECT_FALSE(ExistingD->isUsed(false)); + { +Decl *FromTU = getTuDecl( +"int x = 1; int f() { return x; }", Lang_CXX, "input1.cc"); +auto *FromD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +Import(FromD, Lang_CXX); + } + EXPECT_TRUE(ExistingD->isUsed(false)); +} + +TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag3) { + auto Pattern = varDecl(hasName("a")); + VarDecl *ExistingD; + { +Decl *ToTU = getToTuDecl( +R"( +struct A { + static const int a = 1; +}; +)", Lang_CXX); +ExistingD = FirstDeclMatcher().match(ToTU, Pattern); + } + EXPECT_FALSE(ExistingD->isUsed(false)); + { +Decl *FromTU = getTuDecl( +R"( +struct A { + static const int a = 1; +}; +const int *f() { return &A::a; } // requires storage, + // thus used flag will be set +)", Lang_CXX, "input1.cc"); +auto *FromFunD = FirstDeclMatcher().match( +FromTU, functionDecl(hasName("f"))); +auto *FromD = FirstDeclMatcher().match(FromTU, Pattern); +ASSERT_TRUE(FromD->isUsed(false)); +Import(FromFunD, Lang_CXX); + } + EXPECT_TRUE(ExistingD->isUsed(false)); +} + TEST_P(ASTImporterTestBase, ReimportWithUsedFlag) { auto Pattern = varDecl(hasName("x")); @@ -3244,6 +3293,94 @@ EXPECT_TRUE(ToInitExpr->isGLValue()); } +struct ImportVariables : ASTImporterTestBase {}; + +TEST_P(ImportVariables, ImportOfOneDeclBringsInTheWholeChain) { + Decl *FromTU = getTuDecl( + R"( + struct A { +static const int a = 1 + 2; + }; + const int A::a; + )", Lang_CXX, "input1.cc"); + + auto *FromDWithInit = FirstDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with definition + ASSERT_NE(FromDWithInit, FromDWithDef); + ASSERT_EQ(FromDWithDef->getPreviousDecl(), FromDWithInit); + + auto *ToD0 = cast(Import(FromDWithInit, Lang_CXX11)); + auto *ToD1 = cast(Import(FromDWithDef, Lang_CXX11)); + ASSERT_TRUE(ToD0); + ASSERT_TRUE(ToD1); + EXPECT_NE(ToD0, ToD1); + EXPECT_EQ(ToD1->getPreviousDecl(), ToD0); +} + +TEST_P(ImportVariables, InitAndDefinitionAreInDifferentTUs) { + auto StructA = + R"( + struct A { +static const int a = 1 + 2; + }; + )"; + Decl *ToTU = getToTuDecl(StructA, Lang_CXX); + Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a;", Lang_CXX, + "input1.cc"); + + auto *FromDWithInit = FirstDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with definition + ASSERT_EQ(FromDWithInit, FromDWithDef->getPreviousDecl()); + ASSERT_TRUE(FromDWithInit->getInit()); + ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition()); + ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition()); + ASSERT_FALSE(FromDWithDef->getInit()); + + auto *ToD = FirstDeclMatcher().match( + ToTU, varDecl(hasName("a"))); // Decl with init + ASSERT_TRUE(ToD->getInit()); + ASSERT_FALSE(ToD->getDefinition()); + + auto *ImportedD = cast(Import(FromDWithDef, Lang_CXX11)); + EXPECT_TRUE(ImportedD->getAnyInitializer()); + EXPECT_TRUE(ImportedD->getDefinition()); +} + +TEST_P(ImportVariables, InitAndDefinitionAreInTheFromContext) { + auto StructA = + R"( + struct A { +static const int a; + }; + )"; + Decl *ToTU = getToTuDecl(StructA, Lang_CXX); + Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a = 1 + 2;", + Lang_CXX, "input1.cc"); + + auto *FromDDeclarationOnly = FirstDeclMatcher().match( + FromTU, varDecl(hasName("a"))); + auto *FromDWithDef = LastDeclMatcher().match( +
[PATCH] D49223: [AST] Check described template at structural equivalence check.
martong added a reviewer: a_sidorin. martong added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D49223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong created this revision. martong added reviewers: klimek, aprantl, pcc, sbenza, Prazek, dblaikie, balazske, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Add matchSubtree, so we can traverse on a subtree rooted on a specific node. Currently, we can match **one** node against a matcher, but we will not traverse into the children (this is MatchFinder::match). Or we can traverse through the whole tree rooted at the TUDecl (this is MatchFinder::matchAST). Note, findAll may provide an alternative, but that will traverse throught the whole AST, and that has some weaknesses: https://bugs.llvm.org/show_bug.cgi?id=38318 Repository: rC Clang https://reviews.llvm.org/D49840 Files: include/clang/ASTMatchers/ASTMatchFinder.h lib/ASTMatchers/ASTMatchFinder.cpp unittests/ASTMatchers/ASTMatchersInternalTest.cpp Index: unittests/ASTMatchers/ASTMatchersInternalTest.cpp === --- unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -236,5 +236,53 @@ #endif // _WIN32 + +TEST(Matcher, matchSubtree) { + + std::unique_ptr AST = + clang::tooling::buildASTFromCode( + R"( + template + struct X { +void f() {} +void g() {} + }; + void foo() { + X xc; + xc.f(); + X xi; + } + )"); + ASSERT_TRUE(AST.get()); + + // To get the "f" FunctionDecl inside the instantiation ... + auto FullPattern = + functionDecl(hasName("f") + // ... we must specify the parent. + ,hasParent(classTemplateSpecializationDecl())); + auto *FunD = selectFirst( + "dontcare", match(FullPattern.bind("dontcare"), AST->getASTContext())); + + + auto *SpecD = selectFirst( + "dontcare", match(classTemplateSpecializationDecl().bind("dontcare"), +AST->getASTContext())); + MatchFinder Finder; + struct TestCallback : MatchFinder::MatchCallback { +FunctionDecl *Node = nullptr; +void run(const MatchFinder::MatchResult &Result) override { + Node = + const_cast(Result.Nodes.getNodeAs("")); +} + }; + TestCallback Cb; + + // With matchSubtree we can traverse through the given instantiation. + auto SimplePattern = functionDecl(hasName("f")); + Finder.addMatcher(SimplePattern.bind(""), &Cb); + Finder.matchSubtree(*SpecD, AST->getASTContext()); + EXPECT_EQ(FunD, Cb.Node); +} + } // end namespace ast_matchers } // end namespace clang Index: lib/ASTMatchers/ASTMatchFinder.cpp === --- lib/ASTMatchers/ASTMatchFinder.cpp +++ lib/ASTMatchers/ASTMatchFinder.cpp @@ -1015,6 +1015,32 @@ Visitor.match(Node); } +void MatchFinder::matchSubtree(const clang::ast_type_traits::DynTypedNode &Node, + ASTContext &Context) { + internal::MatchASTVisitor Visitor(&Matchers, Options); + Visitor.set_active_ast_context(&Context); + // FIXME: Improve this with a switch or a visitor pattern. + if (auto *N = Node.get()) { +if (dyn_cast(N)) + Visitor.onStartOfTranslationUnit(); +Visitor.TraverseDecl(const_cast(N)); +if (dyn_cast(N)) + Visitor.onEndOfTranslationUnit(); + } else if (auto *N = Node.get()) { +Visitor.TraverseStmt(const_cast(N)); + } else if (auto *N = Node.get()) { +Visitor.TraverseType(*N); + } else if (auto *N = Node.get()) { +Visitor.TraverseTypeLoc(*N); + } else if (auto *N = Node.get()) { +Visitor.TraverseNestedNameSpecifier(const_cast(N)); + } else if (auto *N = Node.get()) { +Visitor.TraverseNestedNameSpecifierLoc(*N); + } else if (auto *N = Node.get()) { +Visitor.TraverseConstructorInitializer(const_cast(N)); + } +} + void MatchFinder::matchAST(ASTContext &Context) { internal::MatchASTVisitor Visitor(&Matchers, Options); Visitor.set_active_ast_context(&Context); Index: include/clang/ASTMatchers/ASTMatchFinder.h === --- include/clang/ASTMatchers/ASTMatchFinder.h +++ include/clang/ASTMatchers/ASTMatchFinder.h @@ -189,6 +189,15 @@ ASTContext &Context); /// @} + /// \brief Finds all matches in the given subtree rooted at \p Node + /// @{ + template void matchSubtree(const T &Node, ASTContext &Context) { +matchSubtree(clang::ast_type_traits::DynTypedNode::create(Node), Context); + } + void matchSubtree(const clang::ast_type_traits::DynTypedNode &Node, +ASTContext &Context); + /// @} + /// Finds all matches in the given AST. void matchAST(ASTContext &Context); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong added a comment. > Usually we use match(anyOf(node), hasDescendant(node)). Or did I > misunderstand what you want? My understanding is that, the free function template `match` uses `MatchFinder::matchAST`, which will start the traverse from the TranslationUnitDecl. And there is no option to pick a specific node and specify that as the root of the traverse. I'd like an option to be able to start the traverse from a specific node, if it makes sense. Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong added a comment. > MatchFinder::match allows you to match a node. Wrapping your matcher code > with: > auto m = ; > ast_matchers::match(anyOf(m, hashDescendant(m)), node, context); Okay, I understand and accept that. However, I consider that a different level of abstraction. `ast_matchers::match` uses `internal::CollectMatchesCallback` in its implementation. If I already have my own custom MatchCallback implemented then there is no way to achieve the desired behavior. For example, in `ASTImporter` tests we use the following customized callback class: enum class DeclMatcherKind { First, Last }; // Matcher class to retrieve the first/last matched node under a given AST. template class DeclMatcher : public MatchFinder::MatchCallback { NodeType *Node = nullptr; void run(const MatchFinder::MatchResult &Result) override { if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) || MatcherKind == DeclMatcherKind::Last) { Node = const_cast(Result.Nodes.getNodeAs("")); } } public: // Returns the first/last matched node under the tree rooted in `D`. template NodeType *match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; Finder.addMatcher(AMatcher.bind(""), this); Finder.matchAST(D->getASTContext()); assert(Node); return Node; } }; template using LastDeclMatcher = DeclMatcher; template using FirstDeclMatcher = DeclMatcher; And this is how we use it in the tests: Decl *FromTU = getTuDecl("void f(); void f(); void f();", Lang_CXX); auto Pattern = functionDecl(hasName("f")); auto *D0 = FirstDeclMatcher().match(FromTU, Pattern); auto *D2 = LastDeclMatcher().match(FromTU, Pattern); At this point we would like to extend this `DeclMatcher` to be able to match a subtree, and be able to start the traverse from a specific `Decl`, something like this : auto *DV = FirsDeclMatcher().match(D2, SomeOtherPattern); Currently, I don't see how we could do this extension without the proposed `matchSubtree`. (Perhaps, we could refactor our `DeclMatcher` to not use a customized MatchCallback, rather use `ast_matchers::match`, but that sounds like a bad workaround to me.) Hope this makes the the goal of this patch cleaner. Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong added a comment. > Finder.match also has an overload that takes the node. Can you wrap "Pattern" > above in the anyOf(hasDescendant(...), ...) and match on the node instead of > the full AST? Ok, I changed and wrapped the pattern: template NodeType *match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; auto WrappedMatcher = anyOf(AMatcher.bind(""), hasDescendant(AMatcher.bind(""))); Finder.addMatcher(WrappedMatcher, this); // ... } But this results in an ambigous call of `addMatcher` because the compiler cannot choose the appropriate overload: ../../git/llvm/tools/clang/unittests/AST/DeclMatcher.h:36:12: error: call to member function 'addMatcher' is ambiguous Finder.addMatcher(WrappedMatcher, this); ~~~^~ ... ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:149:8: note: candidate function void addMatcher(const DeclarationMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:151:8: note: candidate function void addMatcher(const TypeMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:153:8: note: candidate function void addMatcher(const StatementMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:155:8: note: candidate function void addMatcher(const NestedNameSpecifierMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:157:8: note: candidate function void addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:159:8: note: candidate function void addMatcher(const TypeLocMatcher &NodeMatch, ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:161:8: note: candidate function void addMatcher(const CXXCtorInitializerMatcher &NodeMatch, ^ This seems quite logical to me, since `anyOf` wraps many matchers, which could refer to Nodes with different type. So, I went on and specified which overload to call (note, from this point this is getting hacky and would not consider such an API easy to use): And then I gave up. template NodeType *match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; auto WrappedMatcher = anyOf(AMatcher.bind(""), hasDescendant(AMatcher.bind(""))); auto PtrToMemFun = static_cast(&MatchFinder::addMatcher); (Finder.*PtrToMemFun)(WrappedMatcher, this); This time it turned out that we can't just add a matcker like this because `getMatchers` in `anyOf` would call the private ctor of `Matcher`: ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1339:13: error: calling a private constructor of class 'clang::ast_matchers::internal::Matcher' return {Matcher(std::get(Params))...}; ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1331:16: note: in instantiation of function template specialization 'clang::ast_matchers::internal::VariadicOperatorMatcher, clang::ast_matchers::internal::ArgumentAdaptingMatcherFunc, clang::ast_matchers::internal::TypeList >::Adaptor >::getMatchers' requested here getMatchers(llvm::index_sequence_for())) ^ As an alternative, I tried to call addDynamicMatcher, which resulted in a conversion error: template NodeType *match(const Decl *D, const MatcherType &AMatcher) { MatchFinder Finder; auto WrappedMatcher = anyOf(AMatcher.bind(""), hasDescendant(AMatcher.bind(""))); Finder.addDynamicMatcher(WrappedMatcher, this); ../../git/llvm/tools/clang/unittests/AST/DeclMatcher.h:40:30: error: no viable conversion from 'clang::ast_matchers::internal::VariadicOperatorMatcher, clang::ast_matchers::internal::ArgumentAdaptingMatcherFunc, clang::ast_matchers::internal::TypeList >::Adaptor >' to 'const internal::DynTypedMatcher' Finder.addDynamicMatcher(WrappedMatcher, this); ^~ Perhaps I am doing something wrong because I really could not find a way to add the wrapped matcher. These results motivated me to double check your original idea of using `ast_matchers::match`: Decl *FromTU = getTuDecl("void f();", Lang_CXX); auto Pattern = functionDecl(hasName("f")); match(anyOf(Pattern, hasDescendant(Pattern)), FromTU, FromTU->getASTContext()); This resulted an ambigous call again, but this time the compiler could not resolve the call inside `ast_matchers::match`: ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:301:10: error: call to member function 'addMatcher' is ambiguous Finder.addMatcher(Matcher, &Callback); ~~~^~ Repository: rC Clang https://reviews.llvm.org/D49840 ___ cf
[PATCH] D49840: [AST] Add MatchFinder::matchSubtree
martong added a comment. > If you know the node is a decl, wrapping it in decl() should be enough. > Does this work? > auto WrappedMatcher = decl(anyOf(...)); Unfortunately this gives again the private ctor error (the same error when I called explicitly the overload with the `DeclarationMatcher`): ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1339:13: error: calling a private constructor of class 'clang::ast_matchers::internal::Matcher' return {Matcher(std::get(Params))...}; ^ ../../git/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1331:16: note: in instantiation of function template specialization 'clang::ast_matchers::internal::VariadicOperatorMatcher, clang::ast_matchers::internal::ArgumentAdaptingMatcherFunc, clang::ast_matchers::internal::TypeList >::Adaptor >::getMatchers' requested here getMatchers(llvm::index_sequence_for())) ^ ../../git/llvm/tools/clang/unittests/AST/DeclMatcher.h:35:32: note: in instantiation of function template specialization 'clang::ast_matchers::internal::VariadicOperatorMatcher, clang::ast_matchers::internal::ArgumentAdaptingMatcherFunc, clang::ast_matchers::internal::TypeList >::Adaptor >::operator Matcher' requested here auto WrappedMatcher = decl(anyOf(AMatcher.bind(""), hasDescendant(AMatcher.bind(""; ^ Even if it would work then how could I support nodes other than decls (e.g. stmt())? Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49223: [AST] Check described template at structural equivalence check.
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D49223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2858 + + // Templated declarations should never appear in the enclosing DeclContext. + if (!D->getDescribedVarTemplate()) In case of class templates, the explicit instantiation is the member of the DeclContext. It does not belong to the DeclContext only in case of implicit instantiations. I suppose the same is true for template variables as well. In our code base we fixed it by checking on the TSK_ kind, see https://github.com/Ericsson/clang/pull/270/files. @xazax.hun perhaps we should open source some of our fixes as well? Comment at: lib/AST/ASTImporter.cpp:4455 D2->setLexicalDeclContext(LexicalDC); LexicalDC->addDeclInternal(D2); + This is related to my other comment, perhaps we should not add `D2` to the DeclContext unconditionnally. I think only implicit instantiations should be added. See https://github.com/Ericsson/clang/pull/270/files Repository: rC Clang https://reviews.llvm.org/D43012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl
martong added a comment. Just ignore my previous comments, the issue with explicit instantiations could be fixed in a separate independent patch. All is good. Repository: rC Clang https://reviews.llvm.org/D43012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43967: Add test helper Fixture
martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. [ASTImporter] Add a helper test Fixture, so we can add tests which may check internal attributes of AST nodes. Also it makes possible to import from several "From" contexts. Repository: rC Clang https://reviews.llvm.org/D43967 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index: unittests/AST/DeclMatcher.h === --- /dev/null +++ unittests/AST/DeclMatcher.h @@ -0,0 +1,68 @@ +//===- unittest/AST/DeclMatcher.h - AST unit test support ---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H +#define LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H + +#include "clang/ASTMatchers/ASTMatchFinder.h" + +namespace clang { +namespace ast_matchers { + +enum class DeclMatcherKind { First, Last }; + +// Matcher class to retrieve the first/last matched node under a given AST. +template +class DeclMatcher : public MatchFinder::MatchCallback { + NodeType *Node = nullptr; + void run(const MatchFinder::MatchResult &Result) override { +if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) || +MatcherKind == DeclMatcherKind::Last) { + Node = const_cast(Result.Nodes.getNodeAs("")); +} + } +public: + // Returns the first/last matched node under the tree rooted in `D`. + template + NodeType *match(const Decl *D, const MatcherType &AMatcher) { +MatchFinder Finder; +Finder.addMatcher(AMatcher.bind(""), this); +Finder.matchAST(D->getASTContext()); +assert(Node); +return Node; + } +}; +template +using LastDeclMatcher = DeclMatcher; +template +using FirstDeclMatcher = DeclMatcher; + +template +class DeclCounter : public MatchFinder::MatchCallback { + unsigned count = 0; + void run(const MatchFinder::MatchResult &Result) override { + if(Result.Nodes.getNodeAs("")) { +++count; + } + } +public: + // Returns the number of matched nodes under the tree rooted in `D`. + template + unsigned match(const Decl *D, const MatcherType &AMatcher) { +MatchFinder Finder; +Finder.addMatcher(AMatcher.bind(""), this); +Finder.matchAST(D->getASTContext()); +return count; + } +}; + +} // end namespace ast_matchers +} // end namespace clang + +#endif Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -17,6 +17,8 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/Tooling.h" + +#include "DeclMatcher.h" #include "gtest/gtest.h" namespace clang { @@ -29,7 +31,7 @@ return Lang == Lang_CXX || Lang == Lang_CXX11; } -static RunOptions getRunOptionsForLanguage(Language Lang) { +static ArgVector getBasicRunOptionsForLanguage(Language Lang) { ArgVector BasicArgs; // Test with basic arguments. switch (Lang) { @@ -49,6 +51,11 @@ case Lang_OBJCXX: llvm_unreachable("Not implemented yet!"); } + return BasicArgs; +} + +static RunOptions getRunOptionsForLanguage(Language Lang) { + ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang); // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC // default behaviour. @@ -133,6 +140,146 @@ Verifier, AMatcher)); } +class Fixture : public ::testing::Test { + + const char *const InputFileName = "input.cc"; + const char *const OutputFileName = "output.cc"; + + //Buffers for the contexts, must live in test scope + std::string ToCode; + + struct TU { +std::string Code; +std::string FileName; +std::unique_ptr Unit; +TranslationUnitDecl *TUDecl = nullptr; +TU(StringRef Code, StringRef FileName, ArgVector Args) +: Code(Code), FileName(FileName), + Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args, + this->FileName)), + TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {} + }; + // We may have several From context and related translation units. + std::list FromTUs; + + // Creates a virtual file and assigns that to the To context. If the file + // already exists then the file will not be created again as a duplicate. + void createVirtualFile(StringRef FileName, const std::string &Code) { +assert(ToAST); +ASTContext &ToCtx = ToAST->getASTContext(); +vfs::OverlayFileSystem *OFS = static_cast( +ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get()); +vfs::InMemoryFileSystem *MFS = +st
[PATCH] D43967: Add test helper Fixture
martong added a comment. We add several test cases here, some of them are disabled. We plan to pass the disabled tests in different patches. Repository: rC Clang https://reviews.llvm.org/D43967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits