- Reviewer changes
Hi klimek, sdt, tareqsiraj, arielbernal, Sarcasm,
http://llvm-reviews.chandlerc.com/D1413
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1413?vs=3499&id=3502#toc
Files:
cpp11-migrate/Core/FileOverrides.cpp
cpp11-migrate/Core/FileOverrides.h
cpp11-migrate/Core/Reformatting.cpp
cpp11-migrate/Core/ReplacementsYaml.h
cpp11-migrate/tool/Cpp11Migrate.cpp
test/cpp11-migrate/HeaderReplacements/common.h.yaml
unittests/cpp11-migrate/ReplacementsYamlTest.cpp
Index: cpp11-migrate/Core/FileOverrides.cpp
===================================================================
--- cpp11-migrate/Core/FileOverrides.cpp
+++ cpp11-migrate/Core/FileOverrides.cpp
@@ -30,9 +30,9 @@
void HeaderOverride::recordReplacements(
const clang::tooling::Replacements &Replaces) {
- MigratorDoc.Replacements.resize(Replaces.size());
+ ReplacementsDoc.Replacements.resize(Replaces.size());
std::copy(Replaces.begin(), Replaces.end(),
- MigratorDoc.Replacements.begin());
+ ReplacementsDoc.Replacements.begin());
}
SourceOverrides::SourceOverrides(llvm::StringRef MainFileName,
@@ -93,7 +93,7 @@
// pipeline.
HeaderOverride &HeaderOv = Headers[FileName];
// "Create" HeaderOverride if not already existing
- if (HeaderOv.getFileName().empty())
+ if (HeaderOv.getHeaderPath().empty())
HeaderOv = HeaderOverride(FileName, MainFileName);
HeaderOv.swapContentOverride(ResultBuf);
@@ -150,7 +150,7 @@
assert(!I->second.getContentOverride().empty() &&
"Header override should not be empty!");
SM.overrideFileContents(
- FM.getFile(I->second.getFileName()),
+ FM.getFile(I->second.getHeaderPath()),
llvm::MemoryBuffer::getMemBuffer(I->second.getContentOverride()));
}
}
Index: cpp11-migrate/Core/FileOverrides.h
===================================================================
--- cpp11-migrate/Core/FileOverrides.h
+++ cpp11-migrate/Core/FileOverrides.h
@@ -64,16 +64,15 @@
/// \brief Constructors.
/// @{
HeaderOverride() {}
- HeaderOverride(llvm::StringRef TargetFile,
- llvm::StringRef MainSourceFile) {
- MigratorDoc.TargetFile = TargetFile;
- MigratorDoc.MainSourceFile= MainSourceFile;
+ HeaderOverride(llvm::StringRef HeaderPath,
+ llvm::StringRef MainSourceFile) : HeaderPath(HeaderPath) {
+ ReplacementsDoc.Context = ("Main Source File: " + MainSourceFile).str();
}
/// @}
- /// \brief Getter for FileName.
- llvm::StringRef getFileName() const {
- return MigratorDoc.TargetFile;
+ /// \brief Getter for HeaderPath.
+ llvm::StringRef getHeaderPath() const {
+ return HeaderPath;
}
/// \brief Accessor for ContentOverride.
@@ -88,9 +87,9 @@
/// \brief Swaps the content of ContentOverride with \p S.
void swapContentOverride(std::string &S) { ContentOverride.swap(S); }
- /// \brief Getter for MigratorDoc.
- const MigratorDocument &getMigratorDoc() const {
- return MigratorDoc;
+ /// \brief Getter for ReplacementsDoc.
+ const ReplacementsDocument &getReplacementsDoc() const {
+ return ReplacementsDoc;
}
/// \brief Stores the replacements made by a transform to the header this
@@ -105,7 +104,8 @@
private:
std::string ContentOverride;
ChangedRanges Changes;
- MigratorDocument MigratorDoc;
+ std::string HeaderPath;
+ ReplacementsDocument ReplacementsDoc;
};
/// \brief Container mapping header file names to override information.
Index: cpp11-migrate/Core/Reformatting.cpp
===================================================================
--- cpp11-migrate/Core/Reformatting.cpp
+++ cpp11-migrate/Core/Reformatting.cpp
@@ -47,7 +47,7 @@
I != E; ++I) {
const HeaderOverride &Header = I->getValue();
const tooling::Replacements &HeaderReplaces =
- reformatSingleFile(Header.getFileName(), Header.getChanges(), SM);
+ reformatSingleFile(Header.getHeaderPath(), Header.getChanges(), SM);
Replaces.insert(HeaderReplaces.begin(), HeaderReplaces.end());
}
Overrides.applyReplacements(Replaces, SM);
Index: cpp11-migrate/Core/ReplacementsYaml.h
===================================================================
--- cpp11-migrate/Core/ReplacementsYaml.h
+++ cpp11-migrate/Core/ReplacementsYaml.h
@@ -8,10 +8,8 @@
//===----------------------------------------------------------------------===//
///
/// \file
-/// \brief This file provides functionality to serialize replacements for a
-/// single file. It is used by the C++11 Migrator to store all the changes made
-/// by a single transform to a particular file resulting from migrating a
-/// translation unit of a particular main source file.
+/// \brief This file defines the structure of a YAML document for serializing
+/// replacements.
///
//===----------------------------------------------------------------------===//
@@ -25,14 +23,14 @@
/// \brief The top-level YAML document that contains the details for the
/// replacement.
-struct MigratorDocument {
+struct ReplacementsDocument {
+ /// A freeform chunk of text to describe the context of the replacements in
+ /// this document.
+ std::string Context;
std::vector<clang::tooling::Replacement> Replacements;
- std::string TargetFile;
- std::string MainSourceFile;
};
-// FIXME: Put the YAML support for Replacement into clang::tooling. NOTE: The
-// implementation below doesn't serialize the filename for Replacements.
+// FIXME: Put the YAML support for Replacement into clang::tooling.
LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Replacement)
@@ -83,20 +81,20 @@
static void mapping(IO &Io, clang::tooling::Replacement &R) {
MappingNormalization<NormalizedReplacement, clang::tooling::Replacement>
Keys(Io, R);
+ Io.mapRequired("FilePath", Keys->FilePath);
Io.mapRequired("Offset", Keys->Offset);
Io.mapRequired("Length", Keys->Length);
Io.mapRequired("ReplacementText", Keys->ReplacementText);
}
};
-/// \brief Specialized MappingTraits for MigratorDocument to be converted
+/// \brief Specialized MappingTraits for ReplacementsDocument to be converted
/// to/from a YAML File.
template <>
-struct MappingTraits<MigratorDocument> {
- static void mapping(IO &Io, MigratorDocument &TD) {
- Io.mapRequired("Replacements", TD.Replacements);
- Io.mapRequired("TargetFile", TD.TargetFile);
- Io.mapRequired("MainSourceFile", TD.MainSourceFile);
+struct MappingTraits<ReplacementsDocument> {
+ static void mapping(IO &Io, ReplacementsDocument &Doc) {
+ Io.mapOptional("Context", Doc.Context, std::string());
+ Io.mapRequired("Replacements", Doc.Replacements);
}
};
} // end namespace yaml
Index: cpp11-migrate/tool/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/tool/Cpp11Migrate.cpp
+++ cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -357,8 +357,8 @@
continue;
}
llvm::yaml::Output YAML(ReplacementsFile);
- YAML << const_cast<MigratorDocument &>(
- HeaderI->getValue().getMigratorDoc());
+ YAML << const_cast<ReplacementsDocument &>(
+ HeaderI->getValue().getReplacementsDoc());
} else {
// If -yaml-only was not specified, then change headers on disk.
// FIXME: This is transitional behaviour. Remove this functionality
Index: test/cpp11-migrate/HeaderReplacements/common.h.yaml
===================================================================
--- test/cpp11-migrate/HeaderReplacements/common.h.yaml
+++ test/cpp11-migrate/HeaderReplacements/common.h.yaml
@@ -1,11 +1,12 @@
---
+Context: "Main Source File: $(path)/main.cpp"
Replacements:
- - Offset: 432
+ - FilePath: "$(path)/common.h"
+ Offset: 432
Length: 61
ReplacementText: "(auto & elem : C)"
- - Offset: 506
+ - FilePath: "$(path)/common.h"
+ Offset: 506
Length: 2
ReplacementText: "elem"
-TargetFile: "$(path)/common.h"
-MainSourceFile: "$(path)/main.cpp"
...
Index: unittests/cpp11-migrate/ReplacementsYamlTest.cpp
===================================================================
--- unittests/cpp11-migrate/ReplacementsYamlTest.cpp
+++ unittests/cpp11-migrate/ReplacementsYamlTest.cpp
@@ -21,22 +21,21 @@
using clang::tooling::Replacement;
const std::string TargetFile = "/path/to/common.h";
- const std::string MainSourceFile = "/path/to/source.cpp";
+ const std::string Context = "/path/to/source.cpp";
const unsigned int ReplacementOffset1 = 232;
const unsigned int ReplacementLength1 = 56;
const std::string ReplacementText1 = "(auto & elem : V)";
const unsigned int ReplacementOffset2 = 301;
const unsigned int ReplacementLength2 = 2;
const std::string ReplacementText2 = "elem";
- MigratorDocument Doc;
+ ReplacementsDocument Doc;
Doc.Replacements.push_back(Replacement(TargetFile, ReplacementOffset1,
ReplacementLength1, ReplacementText1));
Doc.Replacements.push_back(Replacement(TargetFile, ReplacementOffset2,
ReplacementLength2, ReplacementText2));
- Doc.TargetFile = TargetFile.c_str();
- Doc.MainSourceFile= MainSourceFile.c_str();
+ Doc.Context = Context;
std::string YamlContent;
llvm::raw_string_ostream YamlContentStream(YamlContent);
@@ -52,22 +51,57 @@
// Read from the YAML file and verify that what was written is exactly what
// we read back.
{
- MigratorDocument DocActual;
+ ReplacementsDocument DocActual;
yaml::Input YAML(YamlContent);
YAML >> DocActual;
ASSERT_NO_ERROR(YAML.error());
- EXPECT_EQ(TargetFile, DocActual.TargetFile);
- EXPECT_EQ(MainSourceFile, DocActual.MainSourceFile);
+ EXPECT_EQ(Context, DocActual.Context);
ASSERT_EQ(2u, DocActual.Replacements.size());
+ EXPECT_EQ(TargetFile, DocActual.Replacements[0].getFilePath());
EXPECT_EQ(ReplacementOffset1, DocActual.Replacements[0].getOffset());
EXPECT_EQ(ReplacementLength1, DocActual.Replacements[0].getLength());
EXPECT_EQ(ReplacementText1,
DocActual.Replacements[0].getReplacementText().str());
+ EXPECT_EQ(TargetFile, DocActual.Replacements[1].getFilePath());
EXPECT_EQ(ReplacementOffset2, DocActual.Replacements[1].getOffset());
EXPECT_EQ(ReplacementLength2, DocActual.Replacements[1].getLength());
EXPECT_EQ(ReplacementText2,
DocActual.Replacements[1].getReplacementText().str());
}
}
+
+TEST(ReplacementsYamlTest, optionalContextWrite) {
+ using clang::tooling::Replacement;
+
+ ReplacementsDocument Doc;
+ Doc.Replacements.push_back(Replacement(/*FilePath=*/"target_file.h",
+ /*Offset=*/1,
+ /*Length=*/10,
+ /*ReplacementText=*/"replacement"));
+ std::string YamlContent;
+ llvm::raw_string_ostream YamlContentStream(YamlContent);
+
+ // Make sure a doc can be written without the context field.
+ yaml::Output YAML(YamlContentStream);
+ YAML << Doc;
+ YamlContentStream.str();
+ ASSERT_NE(YamlContent.length(), 0u);
+ ASSERT_EQ(std::string::npos, YamlContent.find("Context:"));
+}
+
+TEST(ReplacementsYamlTest, optionalContextRead) {
+ // Make sure a doc can be read without the context field.
+ std::string YamlContent = "---\n"
+ "Replacements:\n"
+ " - FilePath: \"target_file.h\"\n"
+ " Offset: 1\n"
+ " Length: 10\n"
+ " ReplacementText: \"replacement\"\n"
+ "...\n";
+ ReplacementsDocument DocActual;
+ yaml::Input YAML(YamlContent);
+ YAML >> DocActual;
+ ASSERT_NO_ERROR(YAML.error());
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
