================
Comment at: cpp11-migrate/Core/FileOverrides.h:62
@@ -60,58 +61,3 @@
-/// \brief Container for storing override information for a single headers.
-class HeaderOverride {
-public:
- /// \brief Constructors.
- /// @{
- HeaderOverride() {}
- HeaderOverride(llvm::StringRef HeaderPath,
- llvm::StringRef MainSourceFile) : HeaderPath(HeaderPath) {
- Replacements.MainSourceFile = MainSourceFile;
- }
- /// @}
-
- /// \brief Getter for HeaderPath.
- llvm::StringRef getHeaderPath() const {
- return HeaderPath;
- }
-
- /// \brief Accessor for ContentOverride.
- /// @{
- llvm::StringRef getContentOverride() const { return ContentOverride; }
- void setContentOverride(const llvm::StringRef S) { ContentOverride = S; }
- /// @}
-
- /// \brief Getter for Changes.
- const ChangedRanges &getChanges() const { return Changes; }
-
- /// \brief Swaps the content of ContentOverride with \p S.
- void swapContentOverride(std::string &S) { ContentOverride.swap(S); }
-
- /// \brief Getter for Replacements.
- const clang::tooling::TranslationUnitReplacements &getReplacements() const {
- return Replacements;
- }
-
- /// \brief Stores the replacements made by a transform to the header this
- /// object represents.
- void recordReplacements(const clang::tooling::Replacements &Replaces);
-
- /// \brief Helper function to adjust the changed ranges.
- void adjustChangedRanges(const clang::tooling::Replacements &Replaces) {
- Changes.adjustChangedRanges(Replaces);
- }
-
-private:
- std::string ContentOverride;
- ChangedRanges Changes;
- std::string HeaderPath;
- clang::tooling::TranslationUnitReplacements Replacements;
-};
-
-/// \brief Container mapping header file names to override information.
-typedef llvm::StringMap<HeaderOverride> HeaderOverrides;
-
-/// \brief Container storing the file content overrides for a source file and
-/// any headers included by the source file either directly or indirectly to
-/// which changes have been made.
-class SourceOverrides {
+// \brief Maintains current state of transformed files and tracks source ranges
+// where changes have been made.
----------------
Doxygen comments here and below.
================
Comment at: cpp11-migrate/Core/FileOverrides.h:109
@@ +108,3 @@
+/// @param[out] Result The resulting unique filename in the same directory as
+/// the header file.
+/// @param[out] Error If an error occurs a description of that error is
----------------
... same directory as the \c MainSourceFile.
================
Comment at: cpp11-migrate/Core/FileOverrides.h:110
@@ +109,3 @@
+/// the header file.
+/// @param[out] Error If an error occurs a description of that error is
+/// appended to this string.
----------------
We do call clear() on Error before appending.
================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:192
@@ +191,3 @@
+ std::string &S = FileStates[FileName];
+ S.clear();
+ llvm::raw_string_ostream StringStream(S);
----------------
Is it necessary to take a reference and clearing the string vs. writing to a temp string and directly assigning to FileStates[FileName]?
================
Comment at: cpp11-migrate/Core/Reformatting.h:35
@@ +34,3 @@
+ /// \param[in] FileState Files to reformat.
+ /// \param SM SourceManager for access to source files.
+ /// \param[out] Replaces Container to store all reformatting replacements.
----------------
Mark SM as [in] to be consistent.
================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:256
@@ +255,3 @@
+
+ return true;
+}
----------------
Shouldn't this be
return replace::writeFiles(DestRewriter);
?
================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:264
@@ +263,3 @@
+ I != E; ++I) {
+ llvm::SmallString<128> ReplacementsHeaderName;
+ llvm::SmallString<64> Error;
----------------
Rename to
ReplacementsFileName
================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:271
@@ +270,3 @@
+ << "\n";
+ Errors = true;
+ }
----------------
Guillaume Papin wrote:
> Shouldn't you skip what's next if the filename wasn't generated correctly?
>
Shouldn't we exit early when generation of the filename fails? Otherwise we will be writing to a invalid file.
================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:279
@@ +278,3 @@
+ llvm::errs() << "Error opening file: " << ErrorInfo << "\n";
+ Errors = true;
+ }
----------------
Guillaume Papin wrote:
> Same here, if there was an error opening the file there is no need to try to write the ReplacementsFile.
Again, exit early if opening the file for writing fails.
================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:367
@@ -294,2 +366,3 @@
+ ChangesReformatter)) {
llvm::errs() << "Header change description files requested for multiple "
"transforms.\nChanges from only one transform can be "
----------------
"Change description file ..."
Since the change descriptions are not header specific anymore.
http://llvm-reviews.chandlerc.com/D1545
BRANCH
driver-model
ARCANIST PROJECT
clang-tools-extra