- Addressed reviewer nits
Hi klimek,
http://llvm-reviews.chandlerc.com/D288
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D288?vs=691&id=710#toc
Files:
cpp11-migrate/CMakeLists.txt
cpp11-migrate/Cpp11Migrate.cpp
cpp11-migrate/LoopConvert/LoopConvert.cpp
cpp11-migrate/LoopConvert/LoopConvert.h
cpp11-migrate/Makefile
cpp11-migrate/Transform.cpp
cpp11-migrate/Transform.h
Index: cpp11-migrate/CMakeLists.txt
===================================================================
--- cpp11-migrate/CMakeLists.txt
+++ cpp11-migrate/CMakeLists.txt
@@ -5,6 +5,7 @@
set (Cpp11MigrateSources
Cpp11Migrate.cpp
Transforms.cpp
+ Transform.cpp
)
# For each transform subdirectory.
Index: cpp11-migrate/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/Cpp11Migrate.cpp
+++ cpp11-migrate/Cpp11Migrate.cpp
@@ -27,27 +27,26 @@
///
//===----------------------------------------------------------------------===//
+#include "Transforms.h"
+#include "Transform.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
-#include "Transforms.h"
-#include "Transform.h"
namespace cl = llvm::cl;
using namespace clang::tooling;
-static cl::opt<RiskLevel>
-MaxRiskLevel("risk", cl::desc("Select a maximum risk level:"),
- cl::values(
- clEnumValN(RL_Safe, "safe", "Only safe transformations"),
- clEnumValN(RL_Reasonable, "reasonable",
- "Enable transformations that might change "
- "semantics (default)"),
- clEnumValN(RL_Risky, "risky",
- "Enable transformations that are likely to "
- "change semantics"),
+static cl::opt<RiskLevel> MaxRiskLevel(
+ "risk", cl::desc("Select a maximum risk level:"),
+ cl::values(clEnumValN(RL_Safe, "safe", "Only safe transformations"),
+ clEnumValN(RL_Reasonable, "reasonable",
+ "Enable transformations that might change "
+ "semantics (default)"),
+ clEnumValN(RL_Risky, "risky",
+ "Enable transformations that are likely to "
+ "change semantics"),
clEnumValEnd),
- cl::init(RL_Reasonable));
+ cl::init(RL_Reasonable));
int main(int argc, const char **argv) {
Transforms TransformManager;
@@ -74,23 +73,53 @@
return 1;
}
+ unsigned int NumFiles = OptionsParser.getSourcePathList().size();
+
+ FileContentsByPath FileStates1, FileStates2,
+ *InputFileStates = &FileStates1, *OutputFileStates = &FileStates2;
+ FileStates1.reserve(NumFiles);
+ FileStates2.reserve(NumFiles);
+
// Apply transforms.
for (Transforms::const_iterator I = TransformManager.begin(),
- E = TransformManager.end(); I != E; ++I) {
- if ((*I)->apply(MaxRiskLevel, OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList()) != 0) {
+ E = TransformManager.end();
+ I != E; ++I) {
+ if ((*I)->apply(*InputFileStates, MaxRiskLevel,
+ OptionsParser.getCompilations(),
+ OptionsParser.getSourcePathList(), *OutputFileStates) !=
+ 0) {
+ // FIXME: Improve ClangTool to not abort if just one file fails.
return 1;
}
+ std::swap(InputFileStates, OutputFileStates);
+ OutputFileStates->clear();
}
+ // Final state of files is pointed at by InputFileStates.
+
// Final Syntax check.
ClangTool EndSyntaxTool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
- if (EndSyntaxTool.run(
- newFrontendActionFactory<clang::SyntaxOnlyAction>()) != 0) {
- // FIXME: Revert changes made to files that fail the syntax test.
+ for (FileContentsByPath::const_iterator I = InputFileStates->begin(),
+ E = InputFileStates->end();
+ I != E; ++I) {
+ EndSyntaxTool.mapVirtualFile(I->first, I->second);
+ }
+
+ if (EndSyntaxTool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>()) !=
+ 0) {
return 1;
}
+ // Syntax check passed, write results to file.
+ for (FileContentsByPath::const_iterator I = InputFileStates->begin(),
+ E = InputFileStates->end();
+ I != E; ++I) {
+ std::string ErrorInfo;
+ llvm::raw_fd_ostream FileStream(I->first.c_str(), ErrorInfo,
+ llvm::raw_fd_ostream::F_Binary);
+ FileStream << I->second;
+ }
+
return 0;
}
Index: cpp11-migrate/LoopConvert/LoopConvert.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopConvert.cpp
+++ cpp11-migrate/LoopConvert/LoopConvert.cpp
@@ -19,15 +19,25 @@
#include "clang/Frontend/FrontendActions.h"
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
+#include "clang/Rewrite/Core/Rewriter.h"
using clang::ast_matchers::MatchFinder;
using namespace clang::tooling;
using namespace clang;
-int LoopConvertTransform::apply(RiskLevel MaxRisk,
+int LoopConvertTransform::apply(const FileContentsByPath &InputStates,
+ RiskLevel MaxRisk,
const CompilationDatabase &Database,
- const std::vector<std::string> &SourcePaths) {
+ const std::vector<std::string> &SourcePaths,
+ FileContentsByPath &ResultStates) {
RefactoringTool LoopTool(Database, SourcePaths);
+
+ for (FileContentsByPath::const_iterator I = InputStates.begin(),
+ E = InputStates.end();
+ I != E; ++I) {
+ LoopTool.mapVirtualFile(I->first, I->second);
+ }
+
StmtAncestorASTVisitor ParentFinder;
StmtGeneratedVarNameMap GeneratedDecls;
ReplacedVarsMap ReplacedVars;
@@ -53,11 +63,19 @@
&RejectedChanges,
MaxRisk, LFK_PseudoArray);
Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
- if (int result = LoopTool.runAndSave(newFrontendActionFactory(&Finder))) {
+
+ if (int result = LoopTool.run(newFrontendActionFactory(&Finder))) {
llvm::errs() << "Error encountered during translation.\n";
return result;
}
+ RewriterContainer Rewrite(LoopTool.getFiles());
+
+ // FIXME: Do something if some replacements didn't get applied?
+ LoopTool.applyAllReplacements(Rewrite.getRewriter());
+
+ collectResults(Rewrite.getRewriter(), ResultStates);
+
if (AcceptedChanges > 0) {
setChangesMade();
}
Index: cpp11-migrate/LoopConvert/LoopConvert.h
===================================================================
--- cpp11-migrate/LoopConvert/LoopConvert.h
+++ cpp11-migrate/LoopConvert/LoopConvert.h
@@ -17,13 +17,19 @@
#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_LOOP_CONVERT_H
#include "Transform.h"
+#include "llvm/Support/Compiler.h" // For LLVM_OVERRIDE
+/// \brief Subclass of Transform that transforms for-loops into range-based
+/// for-loops where possible.
class LoopConvertTransform : public Transform {
public:
- virtual int apply(RiskLevel MaxRiskLEvel,
+ /// \brief \see Transform::run().
+ virtual int apply(const FileContentsByPath &InputStates,
+ RiskLevel MaxRiskLevel,
const clang::tooling::CompilationDatabase &Database,
- const std::vector<std::string> &SourcePaths) LLVM_OVERRIDE;
+ const std::vector<std::string> &SourcePaths,
+ FileContentsByPath &ResultStates) LLVM_OVERRIDE;
};
#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_LOOP_CONVERT_H
Index: cpp11-migrate/Makefile
===================================================================
--- cpp11-migrate/Makefile
+++ cpp11-migrate/Makefile
@@ -17,7 +17,7 @@
include $(CLANG_LEVEL)/../../Makefile.config
-SOURCES = Cpp11Migrate.cpp Transforms.cpp
+SOURCES = Cpp11Migrate.cpp Transforms.cpp Transform.cpp
# For each Transform subdirectory add to SOURCES and BUILT_SOURCES.
# BUILT_SOURCES ensures a subdirectory is created to house object files from
Index: cpp11-migrate/Transform.cpp
===================================================================
--- /dev/null
+++ cpp11-migrate/Transform.cpp
@@ -0,0 +1,35 @@
+#include "Transform.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+
+void collectResults(clang::Rewriter &Rewrite,
+ FileContentsByPath &Results) {
+ for (Rewriter::buffer_iterator I = Rewrite.buffer_begin(),
+ E = Rewrite.buffer_end();
+ I != E; ++I) {
+ const FileEntry *Entry = Rewrite.getSourceMgr().getFileEntryForID(I->first);
+ assert(Entry != 0 && "Expected a FileEntry");
+ assert(Entry->getName() != 0 &&
+ "Unexpected NULL return from FileEntry::getName()");
+
+ FileContentsByPath::value_type ResultEntry;
+
+ ResultEntry.first = Entry->getName();
+
+ // Get a copy of the rewritten buffer from the Rewriter.
+ llvm::raw_string_ostream StringStream(ResultEntry.second);
+ I->second.write(StringStream);
+
+ // Cause results to be written to ResultEntry.second.
+ StringStream.str();
+
+ // FIXME: Use move semantics to avoid copies of the buffer contents if
+ // benchmarking shows the copies are expensive, especially for large source
+ // files.
+ Results.push_back(ResultEntry);
+ }
+}
Index: cpp11-migrate/Transform.h
===================================================================
--- cpp11-migrate/Transform.h
+++ cpp11-migrate/Transform.h
@@ -15,8 +15,18 @@
#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_TRANSFORM_H
#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_TRANSFORM_H
-#include "clang/Tooling/CompilationDatabase.h"
#include <vector>
+#include <string>
+
+// For RewriterContainer
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/raw_ostream.h"
+////
+
/// \brief Description of the riskiness of actions that can be taken by
/// transforms.
@@ -31,13 +41,56 @@
RL_Risky
};
-// Forward Declarations
+// Forward declarations
namespace clang {
namespace tooling {
class CompilationDatabase;
} // namespace tooling
} // namespace clang
+/// \brief First item in the pair is the path of a file and the second is a
+/// buffer with the possibly modified contents of that file.
+typedef std::vector<std::pair<std::string, std::string> > FileContentsByPath;
+
+/// \brief In \p Results place copies of the buffers resulting from applying
+/// all rewrites represented by \p Rewrite.
+///
+/// \p Results is made up of pairs {filename, buffer contents}. Pairs are
+/// simply appended to \p Results.
+void collectResults(clang::Rewriter &Rewrite, FileContentsByPath &Results);
+
+/// \brief Class for containing a Rewriter instance and all of
+/// its lifetime dependencies.
+///
+/// Subclasses of Transform using RefactoringTools will need to create
+/// Rewriters in order to apply Replacements and get the resulting buffer.
+/// Rewriter requires some objects to exist at least as long as it does so this
+/// class contains instances of those objects.
+///
+/// FIXME: These objects should really come from somewhere more global instead
+/// of being recreated for every Transform subclass, especially diagnostics.
+class RewriterContainer {
+public:
+ RewriterContainer(clang::FileManager &Files)
+ : DiagOpts(new clang::DiagnosticOptions()),
+ DiagnosticPrinter(llvm::errs(), DiagOpts.getPtr()),
+ Diagnostics(llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs>(
+ new clang::DiagnosticIDs()),
+ DiagOpts.getPtr(), &DiagnosticPrinter, false),
+ Sources(Diagnostics, Files),
+ Rewrite(Sources, DefaultLangOptions) {}
+
+ clang::Rewriter &getRewriter() { return Rewrite; }
+
+private:
+ clang::LangOptions DefaultLangOptions;
+ llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts;
+ clang::TextDiagnosticPrinter DiagnosticPrinter;
+ clang::DiagnosticsEngine Diagnostics;
+ clang::SourceManager Sources;
+ clang::Rewriter Rewrite;
+};
+
/// \brief Abstract base class for all C++11 migration transforms.
class Transform {
public:
@@ -49,11 +102,16 @@
/// \brief Apply a transform to all files listed in \p SourcePaths.
///
- /// \p Database must contain information for how to compile all files in
- /// \p SourcePaths.
- virtual int apply(RiskLevel MaxRiskLevel,
+ /// \p Database must contain information for how to compile all files in \p
+ /// SourcePaths. \p InputStates contains the file contents of files in \p
+ /// SourcePaths and should take precedence over content of files on disk.
+ /// Upon return, \p ResultStates shall contain the result of performing this
+ /// transform on the files listed in \p SourcePaths.
+ virtual int apply(const FileContentsByPath &InputStates,
+ RiskLevel MaxRiskLevel,
const clang::tooling::CompilationDatabase &Database,
- const std::vector<std::string> &SourcePaths) = 0;
+ const std::vector<std::string> &SourcePaths,
+ FileContentsByPath &ResultStates) = 0;
/// \brief Query if changes were made during the last call to apply().
bool getChangesMade() const { return ChangesMade; }
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
