Skip to content

Commit 96b3d20

Browse files
committedJan 28, 2019
[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 llvm-svn: 352436
1 parent 0068d22 commit 96b3d20

File tree

4 files changed

+241
-21
lines changed

4 files changed

+241
-21
lines changed
 

‎clang/lib/AST/ASTImporter.cpp

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,8 @@ namespace clang {
437437

438438
Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
439439

440+
Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
441+
440442
bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
441443
bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
442444
bool Complain = true);
@@ -2944,6 +2946,17 @@ ASTNodeImporter::FindFunctionTemplateSpecialization(FunctionDecl *FromFD) {
29442946
return FoundSpec;
29452947
}
29462948

2949+
Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
2950+
FunctionDecl *ToFD) {
2951+
if (Stmt *FromBody = FromFD->getBody()) {
2952+
if (ExpectedStmt ToBodyOrErr = import(FromBody))
2953+
ToFD->setBody(*ToBodyOrErr);
2954+
else
2955+
return ToBodyOrErr.takeError();
2956+
}
2957+
return Error::success();
2958+
}
2959+
29472960
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
29482961

29492962
SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
@@ -2967,7 +2980,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
29672980
if (ToD)
29682981
return ToD;
29692982

2970-
const FunctionDecl *FoundByLookup = nullptr;
2983+
FunctionDecl *FoundByLookup = nullptr;
29712984
FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
29722985

29732986
// If this is a function template specialization, then try to find the same
@@ -3038,6 +3051,25 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
30383051
}
30393052
}
30403053

3054+
// We do not allow more than one in-class declaration of a function. This is
3055+
// because AST clients like VTableBuilder asserts on this. VTableBuilder
3056+
// assumes there is only one in-class declaration. Building a redecl
3057+
// chain would result in more than one in-class declaration for
3058+
// overrides (even if they are part of the same redecl chain inside the
3059+
// derived class.)
3060+
if (FoundByLookup) {
3061+
if (auto *MD = dyn_cast<CXXMethodDecl>(FoundByLookup)) {
3062+
if (D->getLexicalDeclContext() == D->getDeclContext()) {
3063+
if (!D->doesThisDeclarationHaveABody())
3064+
return Importer.MapImported(D, FoundByLookup);
3065+
else {
3066+
// Let's continue and build up the redecl chain in this case.
3067+
// FIXME Merge the functions into one decl.
3068+
}
3069+
}
3070+
}
3071+
}
3072+
30413073
DeclarationNameInfo NameInfo(Name, Loc);
30423074
// Import additional name location/type info.
30433075
if (Error Err = ImportDeclarationNameLoc(D->getNameInfo(), NameInfo))
@@ -3199,12 +3231,10 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
31993231
}
32003232

32013233
if (D->doesThisDeclarationHaveABody()) {
3202-
if (Stmt *FromBody = D->getBody()) {
3203-
if (ExpectedStmt ToBodyOrErr = import(FromBody))
3204-
ToFunction->setBody(*ToBodyOrErr);
3205-
else
3206-
return ToBodyOrErr.takeError();
3207-
}
3234+
Error Err = ImportFunctionDeclBody(D, ToFunction);
3235+
3236+
if (Err)
3237+
return std::move(Err);
32083238
}
32093239

32103240
// FIXME: Other bits to merge?

‎clang/test/Analysis/Inputs/ctu-other.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,33 +24,38 @@ namespace embed_ns {
2424
int fens(int x) {
2525
return x - 3;
2626
}
27-
}
27+
} // namespace embed_ns
2828

2929
class embed_cls {
3030
public:
31-
int fecl(int x) {
32-
return x - 7;
33-
}
31+
int fecl(int x);
3432
};
33+
int embed_cls::fecl(int x) {
34+
return x - 7;
3535
}
36+
} // namespace myns
3637

3738
class mycls {
3839
public:
39-
int fcl(int x) {
40-
return x + 5;
41-
}
42-
static int fscl(int x) {
43-
return x + 6;
44-
}
40+
int fcl(int x);
41+
static int fscl(int x);
4542

4643
class embed_cls2 {
4744
public:
48-
int fecl2(int x) {
49-
return x - 11;
50-
}
45+
int fecl2(int x);
5146
};
5247
};
5348

49+
int mycls::fcl(int x) {
50+
return x + 5;
51+
}
52+
int mycls::fscl(int x) {
53+
return x + 6;
54+
}
55+
int mycls::embed_cls2::fecl2(int x) {
56+
return x - 11;
57+
}
58+
5459
namespace chns {
5560
int chf2(int x);
5661

‎clang/test/Analysis/ctu-main.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,6 @@ int main() {
7878
clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
7979

8080
clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
81-
// expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
81+
// expected-warning@Inputs/ctu-other.cpp:80{{REACHABLE}}
8282
MACRODIAG(); // expected-warning{{REACHABLE}}
8383
}

‎clang/unittests/AST/ASTImporterTest.cpp

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,6 +2232,191 @@ TEST_P(ImportFunctions,
22322232
}).match(ToTU, functionDecl()));
22332233
}
22342234

2235+
TEST_P(ImportFunctions, ImportOverriddenMethodTwice) {
2236+
auto Code =
2237+
R"(
2238+
struct B { virtual void f(); };
2239+
struct D:B { void f(); };
2240+
)";
2241+
auto BFP =
2242+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
2243+
auto DFP =
2244+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
2245+
2246+
Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
2247+
auto *DF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
2248+
Import(DF, Lang_CXX);
2249+
2250+
Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
2251+
auto *BF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
2252+
Import(BF, Lang_CXX);
2253+
2254+
auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
2255+
2256+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
2257+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
2258+
}
2259+
2260+
TEST_P(ImportFunctions, ImportOverriddenMethodTwiceDefinitionFirst) {
2261+
auto CodeWithoutDef =
2262+
R"(
2263+
struct B { virtual void f(); };
2264+
struct D:B { void f(); };
2265+
)";
2266+
auto CodeWithDef =
2267+
R"(
2268+
struct B { virtual void f(){}; };
2269+
struct D:B { void f(){}; };
2270+
)";
2271+
auto BFP =
2272+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
2273+
auto DFP =
2274+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
2275+
auto BFDefP = cxxMethodDecl(
2276+
hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
2277+
auto DFDefP = cxxMethodDecl(
2278+
hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
2279+
auto FDefAllP = cxxMethodDecl(hasName("f"), isDefinition());
2280+
2281+
{
2282+
Decl *FromTU = getTuDecl(CodeWithDef, Lang_CXX, "input0.cc");
2283+
auto *FromD = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, DFP);
2284+
Import(FromD, Lang_CXX);
2285+
}
2286+
{
2287+
Decl *FromTU = getTuDecl(CodeWithoutDef, Lang_CXX, "input1.cc");
2288+
auto *FromB = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, BFP);
2289+
Import(FromB, Lang_CXX);
2290+
}
2291+
2292+
auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
2293+
2294+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
2295+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
2296+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 1u);
2297+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDefP), 1u);
2298+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, FDefAllP), 2u);
2299+
}
2300+
2301+
TEST_P(ImportFunctions, ImportOverriddenMethodTwiceOutOfClassDef) {
2302+
auto Code =
2303+
R"(
2304+
struct B { virtual void f(); };
2305+
struct D:B { void f(); };
2306+
void B::f(){};
2307+
)";
2308+
2309+
auto BFP =
2310+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
2311+
auto BFDefP = cxxMethodDecl(
2312+
hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
2313+
auto DFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))),
2314+
unless(isDefinition()));
2315+
2316+
Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
2317+
auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
2318+
Import(D, Lang_CXX);
2319+
2320+
Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
2321+
auto *B = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
2322+
Import(B, Lang_CXX);
2323+
2324+
auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
2325+
2326+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
2327+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 0u);
2328+
2329+
auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
2330+
ToTU, cxxRecordDecl(hasName("B")));
2331+
auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
2332+
auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
2333+
ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
2334+
2335+
// The definition should be out-of-class.
2336+
EXPECT_NE(ToBFInClass, ToBFOutOfClass);
2337+
EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
2338+
ToBFOutOfClass->getLexicalDeclContext());
2339+
EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
2340+
EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
2341+
2342+
// Check that the redecl chain is intact.
2343+
EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
2344+
}
2345+
2346+
TEST_P(ImportFunctions,
2347+
ImportOverriddenMethodTwiceOutOfClassDefInSeparateCode) {
2348+
auto CodeTU0 =
2349+
R"(
2350+
struct B { virtual void f(); };
2351+
struct D:B { void f(); };
2352+
)";
2353+
auto CodeTU1 =
2354+
R"(
2355+
struct B { virtual void f(); };
2356+
struct D:B { void f(); };
2357+
void B::f(){}
2358+
void D::f(){}
2359+
void foo(B &b, D &d) { b.f(); d.f(); }
2360+
)";
2361+
2362+
auto BFP =
2363+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
2364+
auto BFDefP = cxxMethodDecl(
2365+
hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
2366+
auto DFP =
2367+
cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
2368+
auto DFDefP = cxxMethodDecl(
2369+
hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
2370+
auto FooDef = functionDecl(hasName("foo"));
2371+
2372+
{
2373+
Decl *FromTU0 = getTuDecl(CodeTU0, Lang_CXX, "input0.cc");
2374+
auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
2375+
Import(D, Lang_CXX);
2376+
}
2377+
2378+
{
2379+
Decl *FromTU1 = getTuDecl(CodeTU1, Lang_CXX, "input1.cc");
2380+
auto *Foo = FirstDeclMatcher<FunctionDecl>().match(FromTU1, FooDef);
2381+
Import(Foo, Lang_CXX);
2382+
}
2383+
2384+
auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
2385+
2386+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
2387+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
2388+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 0u);
2389+
EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDefP), 0u);
2390+
2391+
auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
2392+
ToTU, cxxRecordDecl(hasName("B")));
2393+
auto *ToD = FirstDeclMatcher<CXXRecordDecl>().match(
2394+
ToTU, cxxRecordDecl(hasName("D")));
2395+
auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
2396+
auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
2397+
ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
2398+
auto *ToDFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, DFP);
2399+
auto *ToDFOutOfClass = LastDeclMatcher<CXXMethodDecl>().match(
2400+
ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
2401+
2402+
// The definition should be out-of-class.
2403+
EXPECT_NE(ToBFInClass, ToBFOutOfClass);
2404+
EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
2405+
ToBFOutOfClass->getLexicalDeclContext());
2406+
EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
2407+
EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
2408+
2409+
EXPECT_NE(ToDFInClass, ToDFOutOfClass);
2410+
EXPECT_NE(ToDFInClass->getLexicalDeclContext(),
2411+
ToDFOutOfClass->getLexicalDeclContext());
2412+
EXPECT_EQ(ToDFOutOfClass->getDeclContext(), ToD);
2413+
EXPECT_EQ(ToDFOutOfClass->getLexicalDeclContext(), ToTU);
2414+
2415+
// Check that the redecl chain is intact.
2416+
EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
2417+
EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
2418+
}
2419+
22352420
struct ImportFriendFunctions : ImportFunctions {};
22362421

22372422
TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {

0 commit comments

Comments
 (0)
Please sign in to comment.