Addressing review comments.
Hi revane, Sarcasm, arielbernal,
http://llvm-reviews.chandlerc.com/D1142
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1142?vs=2945&id=2951#toc
Files:
cpp11-migrate/Core/FileOverrides.cpp
cpp11-migrate/Core/FileOverrides.h
cpp11-migrate/Core/ReplacementsYaml.h
cpp11-migrate/Core/Transform.cpp
cpp11-migrate/tool/Cpp11Migrate.cpp
test/cpp11-migrate/HeaderReplacements/common.cpp
test/cpp11-migrate/HeaderReplacements/common.h
test/cpp11-migrate/HeaderReplacements/common.h.yaml
test/cpp11-migrate/HeaderReplacements/main.cpp
Index: cpp11-migrate/Core/FileOverrides.cpp
===================================================================
--- cpp11-migrate/Core/FileOverrides.cpp
+++ cpp11-migrate/Core/FileOverrides.cpp
@@ -32,19 +32,21 @@
SourceOverrides::SourceOverrides(llvm::StringRef MainFileName)
: MainFileName(MainFileName) {}
-void SourceOverrides::applyReplacements(tooling::Replacements &Replaces) {
+void SourceOverrides::applyReplacements(tooling::Replacements &Replaces,
+ llvm::StringRef TransformName) {
llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts(
new DiagnosticOptions());
DiagnosticsEngine Diagnostics(
llvm::IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()),
DiagOpts.getPtr());
FileManager Files((FileSystemOptions()));
SourceManager SM(Diagnostics, Files);
- applyReplacements(Replaces, SM);
+ applyReplacements(Replaces, SM, TransformName);
}
void SourceOverrides::applyReplacements(tooling::Replacements &Replaces,
- SourceManager &SM) {
+ SourceManager &SM,
+ llvm::StringRef TransformName) {
applyOverrides(SM);
Rewriter Rewrites(SM, LangOptions());
@@ -56,10 +58,6 @@
if (!Success)
llvm::errs() << "error: failed to apply some replacements.";
- applyRewrites(Rewrites);
-}
-
-void SourceOverrides::applyRewrites(Rewriter &Rewrites) {
std::string ResultBuf;
for (Rewriter::buffer_iterator I = Rewrites.buffer_begin(),
@@ -89,10 +87,22 @@
// pipeline.
HeaderOverride &HeaderOv = Headers[FileName];
- HeaderOv.FileOverride.swap(ResultBuf);
+ HeaderOv.swapFileOverride(ResultBuf);
// "Create" HeaderOverride if not already existing
- if (HeaderOv.FileName.empty())
- HeaderOv.FileName = FileName;
+ if (HeaderOv.getFileName().empty())
+ HeaderOv.setFileName(FileName);
+ }
+
+ // Record all replacements to headers.
+ for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
+ I != E; ++I) {
+ if (I->getFilePath() == MainFileName)
+ continue;
+ TransformReplacements TR;
+ TR.TransformID = TransformName;
+ TR.GeneratedReplacements.push_back(*I);
+ HeaderOverride &HeaderOv = Headers[I->getFilePath()];
+ HeaderOv.getTransformReplacementsDoc().Replacements.push_back(TR);
}
}
@@ -105,11 +115,11 @@
for (HeaderOverrides::const_iterator I = Headers.begin(), E = Headers.end();
I != E; ++I) {
- assert(!I->second.FileOverride.empty() &&
+ assert(!I->second.getFileOverride().empty() &&
"Header override should not be empty!");
SM.overrideFileContents(
- FM.getFile(I->second.FileName),
- llvm::MemoryBuffer::getMemBuffer(I->second.FileOverride));
+ FM.getFile(I->second.getFileName()),
+ llvm::MemoryBuffer::getMemBuffer(I->second.getFileOverride()));
}
}
Index: cpp11-migrate/Core/FileOverrides.h
===================================================================
--- cpp11-migrate/Core/FileOverrides.h
+++ cpp11-migrate/Core/FileOverrides.h
@@ -16,6 +16,7 @@
#ifndef CPP11_MIGRATE_FILE_OVERRIDES_H
#define CPP11_MIGRATE_FILE_OVERRIDES_H
+#include "Core/ReplacementsYaml.h"
#include "clang/Tooling/Refactoring.h"
#include "llvm/ADT/StringMap.h"
@@ -31,11 +32,42 @@
/// \brief Container for storing override information for a single headers.
struct HeaderOverride {
- HeaderOverride() {}
- HeaderOverride(llvm::StringRef FileName) : FileName(FileName) {}
+public:
+ /// \brief Constructors.
+ /// @{
+ HeaderOverride() { TransformReplacementsDoc.FileName = ""; }
+ HeaderOverride(llvm::StringRef FileName) : FileName(FileName) {
+ TransformReplacementsDoc.FileName = FileName;
+ }
+ /// @}
+ /// \brief Accessors for FileName.
+ /// @{
+ std::string getFileName() const { return FileName; }
+ void setFileName(const std::string &S) {
+ FileName = S;
+ TransformReplacementsDoc.FileName = S;
+ }
+ /// @}
+
+ /// \brief Accessor for FileOverride.
+ /// @{
+ std::string getFileOverride() const { return FileOverride; }
+ void setFileOverride(const std::string &S) { FileOverride = S; }
+ /// @}
+
+ /// \brief Swaps the content of FileOverride with \param S
+ void swapFileOverride(std::string &S) { FileOverride.swap(S); }
+
+ /// \brief getter for TransformReplacementsDoc.
+ TransformDocument &getTransformReplacementsDoc() const {
+ return const_cast<HeaderOverride*>(this)->TransformReplacementsDoc;
+ }
+
+private:
std::string FileName;
std::string FileOverride;
+ TransformDocument TransformReplacementsDoc;
};
/// \brief Container mapping header file names to override information.
@@ -65,8 +97,10 @@
/// \param Replaces The replacements to apply.
/// \param SM A user provided SourceManager to be used when applying rewrites.
void applyReplacements(clang::tooling::Replacements &Replaces,
- clang::SourceManager &SM);
- void applyReplacements(clang::tooling::Replacements &Replaces);
+ clang::SourceManager &SM,
+ llvm::StringRef TransformName);
+ void applyReplacements(clang::tooling::Replacements &Replaces,
+ llvm::StringRef TransformName);
/// \brief Convenience function for applying this source's overrides to
/// the given SourceManager.
@@ -83,10 +117,6 @@
/// @}
private:
- /// \brief Flatten the Rewriter buffers of \p Rewrite and store results as
- /// file content overrides.
- void applyRewrites(clang::Rewriter &Rewrite);
-
const std::string MainFileName;
std::string MainFileOverride;
HeaderOverrides Headers;
Index: cpp11-migrate/Core/ReplacementsYaml.h
===================================================================
--- /dev/null
+++ cpp11-migrate/Core/ReplacementsYaml.h
@@ -0,0 +1,92 @@
+//===-- Core/ReplacementsYaml.h ---------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file provides structs to store the migrator specific header
+/// replacements and the YAML traits for Replacements.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef CPP11_MIGRATE_REPLACEMENTS_YAML_H
+#define CPP11_MIGRATE_REPLACEMENTS_YAML_H
+
+#include "clang/Tooling/Refactoring.h"
+#include "llvm/Support/YAMLTraits.h"
+#include <vector>
+#include <string>
+
+/// \brief A replacement struct to store the transform ID and the replacement.
+struct TransformReplacements {
+ std::string TransformID;
+ std::vector<clang::tooling::Replacement> GeneratedReplacements;
+};
+
+/// \brief The top-level YAML document that contains the name of the file and
+/// the TransformReplacements.
+struct TransformDocument {
+ std::string FileName;
+ std::vector<TransformReplacements> Replacements;
+};
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Replacement)
+LLVM_YAML_IS_SEQUENCE_VECTOR(TransformReplacements)
+
+namespace llvm {
+namespace yaml {
+
+// ScalarTraits to read/write std::string objects.
+template <>
+struct ScalarTraits<std::string> {
+ static void output(const std::string &Val, void *, llvm::raw_ostream &Out) {
+ Out << Val;
+ }
+
+ static StringRef input(StringRef Scalar, void *, std::string &Val) {
+ Val = Scalar;
+ return StringRef();
+ }
+};
+
+/// \brief Specialized MappingTraits for Repleacements to be converted to/from
+/// a YAML File.
+template <>
+struct MappingTraits<clang::tooling::Replacement> {
+ static void mapping(IO &Io, clang::tooling::Replacement &R) {
+ unsigned int Offset = R.getOffset();
+ unsigned int Length = R.getLength();
+ std::string ReplacementText = R.getReplacementText();
+ Io.mapRequired("Offset", Offset);
+ Io.mapRequired("Length", Length);
+ Io.mapRequired("ReplacementText", ReplacementText);
+ }
+};
+
+/// \brief Specialized MappingTraits for TransformRepleacements to be converted
+/// to/from a YAML File.
+template <>
+struct MappingTraits<TransformReplacements> {
+ static void mapping(IO &Io, TransformReplacements &R) {
+ Io.mapRequired("TransformID", R.TransformID);
+ Io.mapRequired("Replacements", R.GeneratedReplacements);
+ }
+};
+
+/// \brief Specialized MappingTraits for TransformDocument to be converted
+/// to/from a YAML File.
+template <>
+struct MappingTraits<TransformDocument> {
+ static void mapping(IO &Io, TransformDocument &TD) {
+ Io.mapRequired("FileName", TD.FileName);
+ Io.mapRequired("Transforms", TD.Replacements);
+ }
+};
+} // end namespace yaml
+} // end namespace llvm
+
+#endif // CPP11_MIGRATE_REPLACEMENTS_YAML_H
Index: cpp11-migrate/Core/Transform.cpp
===================================================================
--- cpp11-migrate/Core/Transform.cpp
+++ cpp11-migrate/Core/Transform.cpp
@@ -117,7 +117,7 @@
void Transform::handleEndSource() {
if (!getReplacements().empty()) {
SourceOverrides &SO = Overrides->getOrCreate(CurrentSource);
- SO.applyReplacements(getReplacements());
+ SO.applyReplacements(getReplacements(), getName());
}
if (Options().EnableTiming)
Index: cpp11-migrate/tool/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/tool/Cpp11Migrate.cpp
+++ cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -20,6 +20,7 @@
#include "Core/SyntaxCheck.h"
#include "Core/Transform.h"
#include "Core/Transforms.h"
+#include "Core/ReplacementsYaml.h"
#include "LoopConvert/LoopConvert.h"
#include "UseNullptr/UseNullptr.h"
#include "UseAuto/UseAuto.h"
@@ -29,6 +30,7 @@
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/FileSystem.h"
namespace cl = llvm::cl;
using namespace clang;
@@ -209,13 +211,30 @@
for (HeaderOverrides::const_iterator HeaderI = Overrides.headers_begin(),
HeaderE = Overrides.headers_end();
HeaderI != HeaderE; ++HeaderI) {
- assert(!HeaderI->second.FileOverride.empty() &&
+ assert(!HeaderI->second.getFileOverride().empty() &&
"A header override should not be empty");
std::string ErrorInfo;
std::string HeaderFileName = HeaderI->getKey();
llvm::raw_fd_ostream HeaderStream(HeaderFileName.c_str(), ErrorInfo,
llvm::sys::fs::F_Binary);
- HeaderStream << HeaderI->second.FileOverride;
+ HeaderStream << HeaderI->second.getFileOverride();
+
+ // Replacements for header files need to be written in a YAML file for
+ // every transform and will be merged together with an external tool.
+ llvm::SmallString<128> ReplacementsHeaderName;
+ llvm::SmallString<64> Error;
+ bool Result = generateReplacementsFileName(I->getKey(), HeaderFileName,
+ ReplacementsHeaderName, Error);
+ if (!Result)
+ llvm::errs() << "Failed to generate replacements filename:" << Error
+ << "\n";
+
+ ErrorInfo.clear();
+ llvm::raw_fd_ostream ReplacementsFile(ReplacementsHeaderName.c_str(),
+ ErrorInfo,
+ llvm::sys::fs::F_Binary);
+ llvm::yaml::Output YAML(ReplacementsFile);
+ YAML << HeaderI->getValue().getTransformReplacementsDoc();
}
}
Index: test/cpp11-migrate/HeaderReplacements/common.cpp
===================================================================
--- /dev/null
+++ test/cpp11-migrate/HeaderReplacements/common.cpp
@@ -0,0 +1,16 @@
+// This is just a dummy run command to keep lit happy. Tests for this file are
+// in main.cpp
+// RUN: ls
+
+#include "common.h"
+
+void func1(int &I) {
+}
+
+void func2() {
+ vint V1;
+ vint V2;
+ for (vint::iterator I = V1.begin(), E = V1.end(); I != E; ++I) {
+ V2.push_back(*I);
+ }
+}
Index: test/cpp11-migrate/HeaderReplacements/common.h
===================================================================
--- /dev/null
+++ test/cpp11-migrate/HeaderReplacements/common.h
@@ -0,0 +1,18 @@
+#ifndef CPP11_MIGRATE_TEST_HEADER_REPLACEMENTS_COMMON_H
+#define CPP11_MIGRATE_TEST_HEADER_REPLACEMENTS_COMMON_H
+
+#include <vector>
+
+typedef std::vector<int> vint;
+
+void func1(int &I);
+void func2();
+
+void dostuff() {
+ vint V;
+ for (vint::iterator I = V.begin(), E = V.end(); I != E; ++I) {
+ func1(*I);
+ }
+}
+
+#endif // CPP11_MIGRATE_TEST_HEADER_REPLACEMENTS_COMMON_H
Index: test/cpp11-migrate/HeaderReplacements/common.h.yaml
===================================================================
--- /dev/null
+++ test/cpp11-migrate/HeaderReplacements/common.h.yaml
@@ -0,0 +1,14 @@
+---
+FileName: common.h
+Transforms:
+ - TransformID: LoopConvert
+ Replacements:
+ - Offset: 232
+ Length: 56
+ ReplacementText: (auto & elem : V)
+ - TransformID: LoopConvert
+ Replacements:
+ - Offset: 301
+ Length: 2
+ ReplacementText: elem
+...
Index: test/cpp11-migrate/HeaderReplacements/main.cpp
===================================================================
--- /dev/null
+++ test/cpp11-migrate/HeaderReplacements/main.cpp
@@ -0,0 +1,36 @@
+// The following block tests the following:
+// - Only 1 file is generated per translation unit and header file
+// - Replacements are written in YAML that matches the expected YAML file
+// RUN: rm -rf %t/Test
+// RUN: mkdir -p %t/Test
+// RUN: cp %S/main.cpp %S/common.cpp %S/common.h %t/Test
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp %t/Test/common.cpp --
+// Check that only 1 file is generated per translation unit and header file.
+// RUN: ls %t/Test/main.cpp_common.h_*.yaml | wc -l | grep 1
+// RUN: ls %t/Test/common.cpp_common.h_*.yaml | wc -l | grep 1
+// We need to remove the path from FileName in the generated YAML file because it will have a path in the temp directory
+// RUN: sed -i -e 's/^\(FileName:\).*[\/\\]\(.*\)$/\1 \2/g' %t/Test/main.cpp_common.h_*.yaml
+// RUN: sed -i -e 's/^\(FileName:\).*[\/\\]\(.*\)$/\1 \2/g' %t/Test/common.cpp_common.h_*.yaml
+// RUN: diff --ignore-space-change %S/common.h.yaml %t/Test/main.cpp_common.h_*.yaml
+// RUN: diff --ignore-space-change %S/common.h.yaml %t/Test/common.cpp_common.h_*.yaml
+//
+// The following block tests the following:
+// - YAML files are written only when -headers is used
+// RUN: rm -rf %t/Test
+// RUN: mkdir -p %t/Test
+// RUN: cp %S/main.cpp %S/common.cpp %S/common.h %t/Test
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp --
+// RUN: cpp11-migrate -loop-convert %t/Test/common.cpp --
+// Check that only one YAML file is generated from main.cpp and common.h and not from common.cpp and common.h since -header is not specified
+// RUN: ls %t/Test/main.cpp_common.h_*.yaml | wc -l | grep 1
+// RUN: ls %t/Test/common.cpp_common.h_*.yaml | wc -l | grep 0
+// We need to remove the path from FileName in the generated YAML file because it will have a path in the temp directory
+// RUN: sed -i -e 's/^\(FileName:\).*[\/\\]\(.*\)$/\1 \2/g' %t/Test/main.cpp_common.h_*.yaml
+// RUN: diff --ignore-space-change %S/common.h.yaml %t/Test/main.cpp_common.h_*.yaml
+
+#include "common.h"
+
+void test_header_replacement() {
+ dostuff();
+ func2();
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
