r302975 - [ASTImporter] Improve handling of incomplete types
Sean Callanan via cfe-commits
cfe-commits at lists.llvm.org
Fri May 12 17:46:33 PDT 2017
Author: spyffe
Date: Fri May 12 19:46:33 2017
New Revision: 302975
URL: http://llvm.org/viewvc/llvm-project?rev=302975&view=rev
Log:
[ASTImporter] Improve handling of incomplete types
ASTImporter has some bugs when it's importing types
that themselves come from an ExternalASTSource. This
is exposed particularly in the behavior when
comparing complete TagDecls with forward
declarations. This patch does several things:
- Adds a test case making sure that conflicting
forward-declarations are resolved correctly;
- Extends the clang-import-test harness to test
two-level importing, so that we make sure we
complete types when necessary; and
- Fixes a few bugs I found this way. Failure to
complete types was one; however, I also discovered
that complete RecordDecls aren't properly added to
the redecls chain for existing forward
declarations.
Added:
cfe/trunk/test/Import/conflicting-struct/
cfe/trunk/test/Import/conflicting-struct/Inputs/
cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp
cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp
cfe/trunk/test/Import/conflicting-struct/test.cpp
Modified:
cfe/trunk/include/clang/AST/ExternalASTMerger.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
cfe/trunk/lib/AST/ExternalASTMerger.cpp
cfe/trunk/tools/clang-import-test/clang-import-test.cpp
Modified: cfe/trunk/include/clang/AST/ExternalASTMerger.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTMerger.h?rev=302975&r1=302974&r2=302975&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExternalASTMerger.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTMerger.h Fri May 12 19:46:33 2017
@@ -44,6 +44,8 @@ public:
FindExternalLexicalDecls(const DeclContext *DC,
llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
SmallVectorImpl<Decl *> &Result) override;
+
+ void CompleteType(TagDecl *Tag) override;
};
} // end namespace clang
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=302975&r1=302974&r2=302975&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri May 12 19:46:33 2017
@@ -1622,10 +1622,18 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
// We may already have a record of the same name; try to find and match it.
RecordDecl *AdoptDecl = nullptr;
+ RecordDecl *PrevDecl = nullptr;
if (!DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
SmallVector<NamedDecl *, 2> FoundDecls;
DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls);
+
+ if (!FoundDecls.empty()) {
+ // We're going to have to compare D against potentially conflicting Decls, so complete it.
+ if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition())
+ D->getASTContext().getExternalSource()->CompleteType(D);
+ }
+
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (!FoundDecls[I]->isInIdentifierNamespace(IDNS))
continue;
@@ -1652,6 +1660,8 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
}
}
+ PrevDecl = FoundRecord;
+
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
if ((SearchName && !D->isCompleteDefinition())
|| (D->isCompleteDefinition() &&
@@ -1744,6 +1754,10 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
LexicalDC->addDeclInternal(D2);
if (D->isAnonymousStructOrUnion())
D2->setAnonymousStructOrUnion(true);
+ if (PrevDecl) {
+ // FIXME: do this for all Redeclarables, not just RecordDecls.
+ D2->setPreviousDecl(PrevDecl);
+ }
}
Importer.Imported(D, D2);
Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=302975&r1=302974&r2=302975&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original)
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Fri May 12 19:46:33 2017
@@ -855,6 +855,11 @@ static bool IsStructurallyEquivalent(Str
if (CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(D1)) {
if (CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(D2)) {
+ if (D1CXX->hasExternalLexicalStorage() &&
+ !D1CXX->isCompleteDefinition()) {
+ D1CXX->getASTContext().getExternalSource()->CompleteType(D1CXX);
+ }
+
if (D1CXX->getNumBases() != D2CXX->getNumBases()) {
if (Context.Complain) {
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=302975&r1=302974&r2=302975&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp Fri May 12 19:46:33 2017
@@ -178,3 +178,9 @@ void ExternalASTMerger::FindExternalLexi
}
});
}
+
+void ExternalASTMerger::CompleteType(TagDecl *Tag) {
+ SmallVector<Decl *, 0> Result;
+ FindExternalLexicalDecls(Tag, [](Decl::Kind) { return true; }, Result);
+ Tag->setHasExternalLexicalStorage(false);
+}
Added: cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp?rev=302975&view=auto
==============================================================================
--- cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp (added)
+++ cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp Fri May 12 19:46:33 2017
@@ -0,0 +1,6 @@
+class T;
+
+class S {
+ T *t;
+ int a;
+};
Added: cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp?rev=302975&view=auto
==============================================================================
--- cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp (added)
+++ cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp Fri May 12 19:46:33 2017
@@ -0,0 +1,7 @@
+class U {
+ int b;
+};
+
+class T {
+ U u;
+};
Added: cfe/trunk/test/Import/conflicting-struct/test.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/test.cpp?rev=302975&view=auto
==============================================================================
--- cfe/trunk/test/Import/conflicting-struct/test.cpp (added)
+++ cfe/trunk/test/Import/conflicting-struct/test.cpp Fri May 12 19:46:33 2017
@@ -0,0 +1,7 @@
+// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+void expr() {
+ S MyS;
+ T MyT;
+ MyS.a = 3;
+ MyT.u.b = 2;
+}
Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=302975&r1=302974&r2=302975&view=diff
==============================================================================
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Fri May 12 19:46:33 2017
@@ -42,6 +42,10 @@ static llvm::cl::list<std::string>
Imports("import", llvm::cl::ZeroOrMore,
llvm::cl::desc("Path to a file containing declarations to import"));
+static llvm::cl::opt<bool>
+ Direct("direct", llvm::cl::Optional,
+ llvm::cl::desc("Use the parsed declarations without indirection"));
+
static llvm::cl::list<std::string>
ClangArgs("Xcc", llvm::cl::ZeroOrMore,
llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -172,6 +176,14 @@ BuildCompilerInstance(ArrayRef<const cha
return Ins;
}
+std::unique_ptr<CompilerInstance>
+BuildCompilerInstance(ArrayRef<std::string> ClangArgs) {
+ std::vector<const char *> ClangArgv(ClangArgs.size());
+ std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string &s) -> const char * { return s.data(); });
+ return init_convenience::BuildCompilerInstance(ClangArgv);
+}
+
std::unique_ptr<ASTContext>
BuildASTContext(CompilerInstance &CI, SelectorTable &ST, Builtin::Context &BC) {
auto AST = llvm::make_unique<ASTContext>(
@@ -205,6 +217,21 @@ void AddExternalSource(
CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
}
+std::unique_ptr<CompilerInstance> BuildIndirect(std::unique_ptr<CompilerInstance> &CI) {
+ std::vector<const char *> ClangArgv(ClangArgs.size());
+ std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string &s) -> const char * { return s.data(); });
+ std::unique_ptr<CompilerInstance> IndirectCI =
+ init_convenience::BuildCompilerInstance(ClangArgv);
+ auto ST = llvm::make_unique<SelectorTable>();
+ auto BC = llvm::make_unique<Builtin::Context>();
+ std::unique_ptr<ASTContext> AST =
+ init_convenience::BuildASTContext(*IndirectCI, *ST, *BC);
+ IndirectCI->setASTContext(AST.release());
+ AddExternalSource(*IndirectCI, CI);
+ return IndirectCI;
+}
+
llvm::Error ParseSource(const std::string &Path, CompilerInstance &CI,
CodeGenerator &CG) {
SourceManager &SM = CI.getSourceManager();
@@ -231,7 +258,8 @@ Parse(const std::string &Path,
std::unique_ptr<ASTContext> AST =
init_convenience::BuildASTContext(*CI, *ST, *BC);
CI->setASTContext(AST.release());
- AddExternalSource(*CI, Imports);
+ if (Imports.size())
+ AddExternalSource(*CI, Imports);
auto LLVMCtx = llvm::make_unique<llvm::LLVMContext>();
std::unique_ptr<CodeGenerator> CG =
@@ -268,8 +296,21 @@ int main(int argc, const char **argv) {
ImportCIs.push_back(std::move(*ImportCI));
}
}
+ std::vector<std::unique_ptr<CompilerInstance>> IndirectCIs;
+ if (!Direct) {
+ for (auto &ImportCI : ImportCIs) {
+ llvm::Expected<std::unique_ptr<CompilerInstance>> IndirectCI =
+ BuildIndirect(ImportCI);
+ if (auto E = IndirectCI.takeError()) {
+ llvm::errs() << llvm::toString(std::move(E));
+ exit(-1);
+ } else {
+ IndirectCIs.push_back(std::move(*IndirectCI));
+ }
+ }
+ }
llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI =
- Parse(Expression, ImportCIs);
+ Parse(Expression, Direct ? ImportCIs : IndirectCIs);
if (auto E = ExpressionCI.takeError()) {
llvm::errs() << llvm::toString(std::move(E));
exit(-1);
@@ -277,3 +318,4 @@ int main(int argc, const char **argv) {
return 0;
}
}
+
More information about the cfe-commits
mailing list