<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Tue Jun 20 16:06:00 2017<br>
New Revision: 305850<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305850&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305850&view=rev</a><br>
Log:<br>
Preserve CXX method overrides in ASTImporter<br>
<br>
Summary:<br>
The ASTImporter should import CXX method overrides from the source context<br>
when it imports a method decl.<br>
<br>
Reviewers: spyffe, rsmith, doug.gregor<br>
<br>
Reviewed By: spyffe<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D34371" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34371</a><br>
<br>
Modified:<br>
cfe/trunk/lib/AST/ASTDumper.cpp<br>
cfe/trunk/lib/AST/ASTImporter.cpp<br>
cfe/trunk/tools/clang-import-test/clang-import-test.cpp<br>
<br>
Modified: cfe/trunk/lib/AST/ASTDumper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jun 20 16:06:00 2017<br>
@@ -1184,6 +1184,28 @@ void ASTDumper::VisitFunctionDecl(const<br>
I != E; ++I)<br>
dumpCXXCtorInitializer(*I);<br>
<br>
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))<br>
+ if (MD->size_overridden_methods() != 0) {<br></blockquote><div><br>I'd probably make sure CXXMethodDecl has a range-based accessor for overridden_methods and call it once here, check begin != end, then use it in the range-based for later. That way it'd avoid multiple lookups of the CXXMethodDecl in the ASTContext.<br><br>Probably not a big deal/efficiency concern, but just a thought :)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ auto dumpOverride =<br>
+ [=](const CXXMethodDecl *D) {<br>
+ SplitQualType T_split = D->getType().split();<br>
+ OS << D << " " << D->getParent()->getName() << "::"<br>
+ << D->getName() << " '"<br>
+ << QualType::getAsString(T_split) << "'";<br>
+ };<br>
+<br>
+ dumpChild([=] {<br>
+ auto FirstOverrideItr = MD->begin_overridden_methods();<br>
+ OS << "Overrides: [ ";<br>
+ dumpOverride(*FirstOverrideItr);<br></blockquote><div><br>Why is this one ^ pulled out separately from the rest of the loop?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ for (const auto *Override :<br>
+ llvm::make_range(FirstOverrideItr + 1,<br>
+ MD->end_overridden_methods()))<br>
+ dumpOverride(Override);<br>
+ OS << " ]";<br>
+ });<br>
+ }<br>
+<br>
if (D->doesThisDeclarationHaveABody())<br>
dumpStmt(D->getBody());<br>
}<br>
<br>
Modified: cfe/trunk/lib/AST/ASTImporter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jun 20 16:06:00 2017<br>
@@ -319,6 +319,9 @@ namespace clang {<br>
bool ImportArrayChecked(const InContainerTy &InContainer, OIter Obegin) {<br>
return ImportArrayChecked(InContainer.begin(), InContainer.end(), Obegin);<br>
}<br>
+<br>
+ // Importing overrides.<br>
+ void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);<br>
};<br>
}<br>
<br>
@@ -2025,6 +2028,9 @@ Decl *ASTNodeImporter::VisitFunctionDecl<br>
// Add this function to the lexical context.<br>
LexicalDC->addDeclInternal(ToFunction);<br>
<br>
+ if (auto *FromCXXMethod = dyn_cast<CXXMethodDecl>(D))<br>
+ ImportOverrides(cast<CXXMethodDecl>(ToFunction), FromCXXMethod);<br>
+<br>
return ToFunction;<br>
}<br>
<br>
@@ -5499,6 +5505,14 @@ Expr *ASTNodeImporter::VisitSubstNonType<br>
Replacement);<br>
}<br>
<br>
+void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,<br>
+ CXXMethodDecl *FromMethod) {<br>
+ for (auto *FromOverriddenMethod : FromMethod->overridden_methods())<br>
+ ToMethod->addOverriddenMethod(<br>
+ cast<CXXMethodDecl>(Importer.Import(const_cast<CXXMethodDecl*>(<br>
+ FromOverriddenMethod))));<br>
+}<br>
+<br>
ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,<br>
ASTContext &FromContext, FileManager &FromFileManager,<br>
bool MinimalImport)<br>
<br>
Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=305850&r1=305849&r2=305850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=305850&r1=305849&r2=305850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)<br>
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Tue Jun 20 16:06:00 2017<br>
@@ -17,7 +17,9 @@<br>
#include "clang/Basic/TargetInfo.h"<br>
#include "clang/Basic/TargetOptions.h"<br>
#include "clang/CodeGen/ModuleBuilder.h"<br>
+#include "clang/Frontend/ASTConsumers.h"<br>
#include "clang/Frontend/CompilerInstance.h"<br>
+#include "clang/Frontend/MultiplexConsumer.h"<br>
#include "clang/Frontend/TextDiagnosticBuffer.h"<br>
#include "clang/Lex/Lexer.h"<br>
#include "clang/Lex/Preprocessor.h"<br>
@@ -51,6 +53,10 @@ static llvm::cl::list<std::string><br>
llvm::cl::desc("Argument to pass to the CompilerInvocation"),<br>
llvm::cl::CommaSeparated);<br>
<br>
+static llvm::cl::opt<bool><br>
+DumpAST("dump-ast", llvm::cl::init(false),<br>
+ llvm::cl::desc("Dump combined AST"));<br>
+<br>
namespace init_convenience {<br>
class TestDiagnosticConsumer : public DiagnosticConsumer {<br>
private:<br>
@@ -233,7 +239,7 @@ std::unique_ptr<CompilerInstance> BuildI<br>
}<br>
<br>
llvm::Error ParseSource(const std::string &Path, CompilerInstance &CI,<br>
- CodeGenerator &CG) {<br>
+ ASTConsumer &Consumer) {<br>
SourceManager &SM = CI.getSourceManager();<br>
const FileEntry *FE = CI.getFileManager().getFile(Path);<br>
if (!FE) {<br>
@@ -241,13 +247,14 @@ llvm::Error ParseSource(const std::strin<br>
llvm::Twine("Couldn't open ", Path), std::error_code());<br>
}<br>
SM.setMainFileID(SM.createFileID(FE, SourceLocation(), SrcMgr::C_User));<br>
- ParseAST(CI.getPreprocessor(), &CG, CI.getASTContext());<br>
+ ParseAST(CI.getPreprocessor(), &Consumer, CI.getASTContext());<br>
return llvm::Error::success();<br>
}<br>
<br>
llvm::Expected<std::unique_ptr<CompilerInstance>><br>
Parse(const std::string &Path,<br>
- llvm::ArrayRef<std::unique_ptr<CompilerInstance>> Imports) {<br>
+ llvm::ArrayRef<std::unique_ptr<CompilerInstance>> Imports,<br>
+ bool ShouldDumpAST) {<br>
std::vector<const char *> ClangArgv(ClangArgs.size());<br>
std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),<br>
[](const std::string &s) -> const char * { return s.data(); });<br>
@@ -261,14 +268,20 @@ Parse(const std::string &Path,<br>
if (Imports.size())<br>
AddExternalSource(*CI, Imports);<br>
<br>
+ std::vector<std::unique_ptr<ASTConsumer>> ASTConsumers;<br>
+<br>
auto LLVMCtx = llvm::make_unique<llvm::LLVMContext>();<br>
- std::unique_ptr<CodeGenerator> CG =<br>
- init_convenience::BuildCodeGen(*CI, *LLVMCtx);<br>
- CG->Initialize(CI->getASTContext());<br>
+ ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));<br>
+<br>
+ if (ShouldDumpAST)<br>
+ ASTConsumers.push_back(CreateASTDumper("", true, false, false));<br>
<br>
CI->getDiagnosticClient().BeginSourceFile(CI->getLangOpts(),<br>
&CI->getPreprocessor());<br>
- if (llvm::Error PE = ParseSource(Path, *CI, *CG)) {<br>
+ MultiplexConsumer Consumers(std::move(ASTConsumers));<br>
+ Consumers.Initialize(CI->getASTContext());<br>
+<br>
+ if (llvm::Error PE = ParseSource(Path, *CI, Consumers)) {<br>
return std::move(PE);<br>
}<br>
CI->getDiagnosticClient().EndSourceFile();<br>
@@ -288,7 +301,8 @@ int main(int argc, const char **argv) {<br>
llvm::cl::ParseCommandLineOptions(argc, argv);<br>
std::vector<std::unique_ptr<CompilerInstance>> ImportCIs;<br>
for (auto I : Imports) {<br>
- llvm::Expected<std::unique_ptr<CompilerInstance>> ImportCI = Parse(I, {});<br>
+ llvm::Expected<std::unique_ptr<CompilerInstance>> ImportCI =<br>
+ Parse(I, {}, false);<br>
if (auto E = ImportCI.takeError()) {<br>
llvm::errs() << llvm::toString(std::move(E));<br>
exit(-1);<br>
@@ -310,7 +324,7 @@ int main(int argc, const char **argv) {<br>
}<br>
}<br>
llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI =<br>
- Parse(Expression, Direct ? ImportCIs : IndirectCIs);<br>
+ Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);<br>
if (auto E = ExpressionCI.takeError()) {<br>
llvm::errs() << llvm::toString(std::move(E));<br>
exit(-1);<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>