r352436 - [ASTImporter] Fix handling of overriden methods during ASTImport
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 28 13:55:33 PST 2019
Author: shafik
Date: Mon Jan 28 13:55:33 2019
New Revision: 352436
URL: http://llvm.org/viewvc/llvm-project?rev=352436&view=rev
Log:
[ASTImporter] Fix handling of overriden methods during ASTImport
Summary:
When importing classes we may add a CXXMethodDecl more than once to a CXXRecordDecl when handling overrides. This patch will fix the cases we currently know about and handle the case where we are only dealing with declarations.
Differential Revision: https://reviews.llvm.org/D56936
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/ctu-main.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=352436&r1=352435&r2=352436&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Jan 28 13:55:33 2019
@@ -437,6 +437,8 @@ namespace clang {
Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
+ Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
+
bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
@@ -2944,6 +2946,17 @@ ASTNodeImporter::FindFunctionTemplateSpe
return FoundSpec;
}
+Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
+ FunctionDecl *ToFD) {
+ if (Stmt *FromBody = FromFD->getBody()) {
+ if (ExpectedStmt ToBodyOrErr = import(FromBody))
+ ToFD->setBody(*ToBodyOrErr);
+ else
+ return ToBodyOrErr.takeError();
+ }
+ return Error::success();
+}
+
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
@@ -2967,7 +2980,7 @@ ExpectedDecl ASTNodeImporter::VisitFunct
if (ToD)
return ToD;
- const FunctionDecl *FoundByLookup = nullptr;
+ FunctionDecl *FoundByLookup = nullptr;
FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
// If this is a function template specialization, then try to find the same
@@ -3038,6 +3051,25 @@ ExpectedDecl ASTNodeImporter::VisitFunct
}
}
+ // We do not allow more than one in-class declaration of a function. This is
+ // because AST clients like VTableBuilder asserts on this. VTableBuilder
+ // assumes there is only one in-class declaration. Building a redecl
+ // chain would result in more than one in-class declaration for
+ // overrides (even if they are part of the same redecl chain inside the
+ // derived class.)
+ if (FoundByLookup) {
+ if (auto *MD = dyn_cast<CXXMethodDecl>(FoundByLookup)) {
+ if (D->getLexicalDeclContext() == D->getDeclContext()) {
+ if (!D->doesThisDeclarationHaveABody())
+ return Importer.MapImported(D, FoundByLookup);
+ else {
+ // Let's continue and build up the redecl chain in this case.
+ // FIXME Merge the functions into one decl.
+ }
+ }
+ }
+ }
+
DeclarationNameInfo NameInfo(Name, Loc);
// Import additional name location/type info.
if (Error Err = ImportDeclarationNameLoc(D->getNameInfo(), NameInfo))
@@ -3199,12 +3231,10 @@ ExpectedDecl ASTNodeImporter::VisitFunct
}
if (D->doesThisDeclarationHaveABody()) {
- if (Stmt *FromBody = D->getBody()) {
- if (ExpectedStmt ToBodyOrErr = import(FromBody))
- ToFunction->setBody(*ToBodyOrErr);
- else
- return ToBodyOrErr.takeError();
- }
+ Error Err = ImportFunctionDeclBody(D, ToFunction);
+
+ if (Err)
+ return std::move(Err);
}
// FIXME: Other bits to merge?
Modified: cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/ctu-other.cpp?rev=352436&r1=352435&r2=352436&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/ctu-other.cpp (original)
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.cpp Mon Jan 28 13:55:33 2019
@@ -24,33 +24,38 @@ namespace embed_ns {
int fens(int x) {
return x - 3;
}
-}
+} // namespace embed_ns
class embed_cls {
public:
- int fecl(int x) {
- return x - 7;
- }
+ int fecl(int x);
};
+int embed_cls::fecl(int x) {
+ return x - 7;
}
+} // namespace myns
class mycls {
public:
- int fcl(int x) {
- return x + 5;
- }
- static int fscl(int x) {
- return x + 6;
- }
+ int fcl(int x);
+ static int fscl(int x);
class embed_cls2 {
public:
- int fecl2(int x) {
- return x - 11;
- }
+ int fecl2(int x);
};
};
+int mycls::fcl(int x) {
+ return x + 5;
+}
+int mycls::fscl(int x) {
+ return x + 6;
+}
+int mycls::embed_cls2::fecl2(int x) {
+ return x - 11;
+}
+
namespace chns {
int chf2(int x);
Modified: cfe/trunk/test/Analysis/ctu-main.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctu-main.cpp?rev=352436&r1=352435&r2=352436&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctu-main.cpp (original)
+++ cfe/trunk/test/Analysis/ctu-main.cpp Mon Jan 28 13:55:33 2019
@@ -78,6 +78,6 @@ int main() {
clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
- // expected-warning at Inputs/ctu-other.cpp:75{{REACHABLE}}
+ // expected-warning at Inputs/ctu-other.cpp:80{{REACHABLE}}
MACRODIAG(); // expected-warning{{REACHABLE}}
}
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=352436&r1=352435&r2=352436&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Jan 28 13:55:33 2019
@@ -2232,6 +2232,191 @@ TEST_P(ImportFunctions,
}).match(ToTU, functionDecl()));
}
+TEST_P(ImportFunctions, ImportOverriddenMethodTwice) {
+ auto Code =
+ R"(
+ struct B { virtual void f(); };
+ struct D:B { void f(); };
+ )";
+ auto BFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+ auto DFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+
+ Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
+ auto *DF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+ Import(DF, Lang_CXX);
+
+ Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+ auto *BF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
+ Import(BF, Lang_CXX);
+
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
+}
+
+TEST_P(ImportFunctions, ImportOverriddenMethodTwiceDefinitionFirst) {
+ auto CodeWithoutDef =
+ R"(
+ struct B { virtual void f(); };
+ struct D:B { void f(); };
+ )";
+ auto CodeWithDef =
+ R"(
+ struct B { virtual void f(){}; };
+ struct D:B { void f(){}; };
+ )";
+ auto BFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+ auto DFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+ auto BFDefP = cxxMethodDecl(
+ hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+ auto DFDefP = cxxMethodDecl(
+ hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
+ auto FDefAllP = cxxMethodDecl(hasName("f"), isDefinition());
+
+ {
+ Decl *FromTU = getTuDecl(CodeWithDef, Lang_CXX, "input0.cc");
+ auto *FromD = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, DFP);
+ Import(FromD, Lang_CXX);
+ }
+ {
+ Decl *FromTU = getTuDecl(CodeWithoutDef, Lang_CXX, "input1.cc");
+ auto *FromB = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, BFP);
+ Import(FromB, Lang_CXX);
+ }
+
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDefP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, FDefAllP), 2u);
+}
+
+TEST_P(ImportFunctions, ImportOverriddenMethodTwiceOutOfClassDef) {
+ auto Code =
+ R"(
+ struct B { virtual void f(); };
+ struct D:B { void f(); };
+ void B::f(){};
+ )";
+
+ auto BFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+ auto BFDefP = cxxMethodDecl(
+ hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+ auto DFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))),
+ unless(isDefinition()));
+
+ Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
+ auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+ Import(D, Lang_CXX);
+
+ Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+ auto *B = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
+ Import(B, Lang_CXX);
+
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 0u);
+
+ auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("B")));
+ auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
+ auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
+ ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+
+ // The definition should be out-of-class.
+ EXPECT_NE(ToBFInClass, ToBFOutOfClass);
+ EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
+ ToBFOutOfClass->getLexicalDeclContext());
+ EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
+ EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
+
+ // Check that the redecl chain is intact.
+ EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
+}
+
+TEST_P(ImportFunctions,
+ ImportOverriddenMethodTwiceOutOfClassDefInSeparateCode) {
+ auto CodeTU0 =
+ R"(
+ struct B { virtual void f(); };
+ struct D:B { void f(); };
+ )";
+ auto CodeTU1 =
+ R"(
+ struct B { virtual void f(); };
+ struct D:B { void f(); };
+ void B::f(){}
+ void D::f(){}
+ void foo(B &b, D &d) { b.f(); d.f(); }
+ )";
+
+ auto BFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+ auto BFDefP = cxxMethodDecl(
+ hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+ auto DFP =
+ cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+ auto DFDefP = cxxMethodDecl(
+ hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
+ auto FooDef = functionDecl(hasName("foo"));
+
+ {
+ Decl *FromTU0 = getTuDecl(CodeTU0, Lang_CXX, "input0.cc");
+ auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+ Import(D, Lang_CXX);
+ }
+
+ {
+ Decl *FromTU1 = getTuDecl(CodeTU1, Lang_CXX, "input1.cc");
+ auto *Foo = FirstDeclMatcher<FunctionDecl>().match(FromTU1, FooDef);
+ Import(Foo, Lang_CXX);
+ }
+
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 0u);
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDefP), 0u);
+
+ auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("B")));
+ auto *ToD = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("D")));
+ auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
+ auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
+ ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+ auto *ToDFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, DFP);
+ auto *ToDFOutOfClass = LastDeclMatcher<CXXMethodDecl>().match(
+ ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+
+ // The definition should be out-of-class.
+ EXPECT_NE(ToBFInClass, ToBFOutOfClass);
+ EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
+ ToBFOutOfClass->getLexicalDeclContext());
+ EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
+ EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
+
+ EXPECT_NE(ToDFInClass, ToDFOutOfClass);
+ EXPECT_NE(ToDFInClass->getLexicalDeclContext(),
+ ToDFOutOfClass->getLexicalDeclContext());
+ EXPECT_EQ(ToDFOutOfClass->getDeclContext(), ToD);
+ EXPECT_EQ(ToDFOutOfClass->getLexicalDeclContext(), ToTU);
+
+ // Check that the redecl chain is intact.
+ EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
+ EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
+}
+
struct ImportFriendFunctions : ImportFunctions {};
TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
More information about the cfe-commits
mailing list