- Addressed reviewer comments
- Also changed -format-style into -format which now has an optional value
that
defaults to the llvm style.
Hi tareqsiraj, arielbernal, Sarcasm, klimek,
http://llvm-reviews.chandlerc.com/D1730
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1730?vs=4409&id=4423#toc
Files:
clang-apply-replacements/CMakeLists.txt
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
test/clang-apply-replacements/Inputs/format/no.cpp
test/clang-apply-replacements/Inputs/format/no.yaml
test/clang-apply-replacements/Inputs/format/yes.cpp
test/clang-apply-replacements/Inputs/format/yes.yaml
test/clang-apply-replacements/format.cpp
Index: clang-apply-replacements/CMakeLists.txt
===================================================================
--- clang-apply-replacements/CMakeLists.txt
+++ clang-apply-replacements/CMakeLists.txt
@@ -13,6 +13,7 @@
clangTooling
clangBasic
clangRewriteFrontend
+ clangFormat
)
include_directories(
Index: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
===================================================================
--- clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
+++ clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
@@ -28,8 +28,15 @@
class DiagnosticsEngine;
class Rewriter;
+namespace format {
+struct FormatStyle;
+} // end namespace format
+
namespace replace {
+/// \brief Collection of source ranges.
+typedef std::vector<clang::tooling::Range> RangeVec;
+
/// \brief Collection of TranslationUnitReplacements.
typedef std::vector<clang::tooling::TranslationUnitReplacements>
TUReplacements;
@@ -91,6 +98,35 @@
bool applyReplacements(const FileToReplacementsMap &GroupedReplacements,
clang::Rewriter &Rewrites);
+/// \brief Given a collection of Replacements for a single file, produces a list
+/// of source ranges that enclose those Replacements.
+///
+/// \param[in] Replacements Replacements from a single file.
+/// \param[out] ChangedRanges Collection of source ranges that enclose all
+/// given Replacements. Ranges do not overlap and will not be directly adjacent
+/// as such ranges are combined into one.
+void calculateChangedRanges(
+ const std::vector<clang::tooling::Replacement> &Replacements,
+ RangeVec &ChangedRanges);
+
+/// \brief Applies source-code formatting to the given source ranges of the
+/// given file.
+///
+/// This function uses \c clang::format::reformat() so the pre/post conditions
+/// of that function apply here as well. In particular, code outside of the
+/// given ranges may be formatted if it's part of the syntactic unit being
+/// formatted.
+///
+/// \param[in] FileName Name of the file being formatted.
+/// \param[in] Ranges Sequence of source ranges to format.
+/// \param[in] SM SourceManager for interpreting source ranges.
+/// \param[in] Style Coding style to apply.
+/// \param[out] Replacements New replacements that will reformat code when
+/// applied.
+void reformatRanges(const StringRef FileName, const RangeVec &Ranges,
+ clang::SourceManager &SM, clang::format::FormatStyle Style,
+ std::vector<clang::tooling::Replacement> &Replacements);
+
/// \brief Write the contents of \c FileContents to disk. Keys of the map are
/// filenames and values are the new contents for those files.
///
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -17,6 +17,8 @@
#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
+#include "clang/Lex/Lexer.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/ReplacementsYaml.h"
#include "llvm/ADT/ArrayRef.h"
@@ -35,6 +37,52 @@
namespace replace {
+/// \brief Comparator to be able to order tooling::Range based on their offset.
+static bool rangeLess(clang::tooling::Range A, clang::tooling::Range B) {
+ if (A.getOffset() == B.getOffset())
+ return A.getLength() < B.getLength();
+ return A.getOffset() < B.getOffset();
+}
+
+namespace {
+
+/// \brief Functor that returns the given range without its overlaps with the
+/// replacement given in the constructor.
+struct RangeReplacedAdjuster {
+ RangeReplacedAdjuster(const tooling::Replacement &Replace)
+ : Replace(Replace.getOffset(), Replace.getLength()),
+ ReplaceNewSize(Replace.getReplacementText().size()) {}
+
+ tooling::Range operator()(tooling::Range Range) const {
+ if (!Range.overlapsWith(Replace))
+ return Range;
+ // range inside replacement -> make the range length null
+ if (Replace.contains(Range))
+ return tooling::Range(Range.getOffset(), 0);
+ // replacement inside range -> resize the range
+ if (Range.contains(Replace)) {
+ int Difference = ReplaceNewSize - Replace.getLength();
+ return tooling::Range(Range.getOffset(), Range.getLength() + Difference);
+ }
+ // beginning of the range replaced -> truncate range beginning
+ if (Range.getOffset() > Replace.getOffset()) {
+ unsigned ReplaceEnd = Replace.getOffset() + Replace.getLength();
+ unsigned RangeEnd = Range.getOffset() + Range.getLength();
+ return tooling::Range(ReplaceEnd, RangeEnd - ReplaceEnd);
+ }
+ // end of the range replaced -> truncate range end
+ if (Range.getOffset() < Replace.getOffset())
+ return tooling::Range(Range.getOffset(),
+ Replace.getOffset() - Range.getOffset());
+ llvm_unreachable("conditions not handled properly");
+ }
+
+ const tooling::Range Replace;
+ const unsigned ReplaceNewSize;
+};
+
+} // end anonymous namespace
+
llvm::error_code
collectReplacementsFromDirectory(const llvm::StringRef Directory,
TUReplacements &TUs,
@@ -212,6 +260,95 @@
return true;
}
+static void coalesceRanges(RangeVec &Ranges) {
+ // Sort the ranges by offset and then for each group of adjacent/overlapping
+ // ranges the first one in the group is extended to cover the whole group.
+ std::sort(Ranges.begin(), Ranges.end(), &rangeLess);
+ RangeVec::iterator FirstInGroup = Ranges.begin();
+ assert(!Ranges.empty() && "unexpected empty vector");
+ for (RangeVec::iterator I = Ranges.begin() + 1, E = Ranges.end(); I != E;
+ ++I) {
+ unsigned GroupEnd = FirstInGroup->getOffset() + FirstInGroup->getLength();
+
+ // no contact
+ if (I->getOffset() > GroupEnd)
+ FirstInGroup = I;
+ else {
+ unsigned GrpBegin = FirstInGroup->getOffset();
+ unsigned GrpEnd = std::max(GroupEnd, I->getOffset() + I->getLength());
+ *FirstInGroup = tooling::Range(GrpBegin, GrpEnd - GrpBegin);
+ }
+ }
+
+ // Remove the ranges that are covered by the first member of the group.
+ Ranges.erase(std::unique(Ranges.begin(), Ranges.end(),
+ std::mem_fun_ref(&tooling::Range::contains)),
+ Ranges.end());
+}
+
+void calculateChangedRanges(
+ const std::vector<clang::tooling::Replacement> &Replaces,
+ RangeVec &ChangedRanges) {
+
+ // First adjust existing ranges in case they overlap with the replacements.
+ for (std::vector<clang::tooling::Replacement>::const_iterator
+ I = Replaces.begin(),
+ E = Replaces.end();
+ I != E; ++I) {
+ const tooling::Replacement &Replace = *I;
+
+ std::transform(ChangedRanges.begin(), ChangedRanges.end(),
+ ChangedRanges.begin(), RangeReplacedAdjuster(Replace));
+ }
+
+ // Then shift existing ranges to reflect the new positions.
+ for (RangeVec::iterator I = ChangedRanges.begin(), E = ChangedRanges.end();
+ I != E; ++I) {
+ unsigned ShiftedOffset =
+ tooling::shiftedCodePosition(Replaces, I->getOffset());
+ *I = tooling::Range(ShiftedOffset, I->getLength());
+ }
+
+ // Then generate the new ranges from the replacements.
+ for (std::vector<clang::tooling::Replacement>::const_iterator
+ I = Replaces.begin(),
+ E = Replaces.end();
+ I != E; ++I) {
+ const tooling::Replacement &R = *I;
+ unsigned Offset = tooling::shiftedCodePosition(Replaces, R.getOffset());
+ unsigned Length = R.getReplacementText().size();
+
+ ChangedRanges.push_back(tooling::Range(Offset, Length));
+ }
+
+ coalesceRanges(ChangedRanges);
+}
+
+void reformatRanges(const llvm::StringRef FileName, const RangeVec &Ranges,
+ clang::SourceManager &SM, clang::format::FormatStyle Style,
+ std::vector<tooling::Replacement> &Replacements) {
+ const FileEntry *Entry = SM.getFileManager().getFile(FileName);
+ assert(Entry && "expected an existing file");
+
+ FileID ID = SM.translateFile(Entry);
+ if (ID.isInvalid())
+ ID = SM.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
+
+ std::vector<CharSourceRange> ReformatRanges;
+ SourceLocation StartOfFile = SM.getLocForStartOfFile(ID);
+ for (RangeVec::const_iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+ ++I) {
+ SourceLocation Start = StartOfFile.getLocWithOffset(I->getOffset());
+ SourceLocation End = Start.getLocWithOffset(I->getLength());
+ ReformatRanges.push_back(CharSourceRange::getCharRange(Start, End));
+ }
+
+ Lexer Lex(ID, SM.getBuffer(ID), SM, getFormattingLangOpts(Style.Standard));
+ const tooling::Replacements &R =
+ format::reformat(Style, Lex, SM, ReformatRanges);
+ std::copy(R.begin(), R.end(), std::back_inserter(Replacements));
+}
+
bool writeFiles(const clang::Rewriter &Rewrites) {
for (Rewriter::const_buffer_iterator BufferI = Rewrites.buffer_begin(),
Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===================================================================
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -17,6 +17,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "llvm/Support/CommandLine.h"
@@ -33,6 +34,61 @@
"merging/replacing."),
cl::init(false));
+static cl::opt<std::string> FormatStyleOpt(
+ "format",
+ cl::desc(
+ "Enable formatting of code code changed by applying replacements.\n"
+ "<style> can be either a builtin style supported by clang-format,\n"
+ "or a YAML config file (see clang-format -dump-config).\n"
+ "The default style is 'llvm'"),
+ cl::ValueOptional, cl::value_desc("style"));
+
+/// \brief Fills in \c Style with information from the command-line option
+/// "format-style".
+///
+/// Upon error, print a diagnostic error message.
+///
+/// \param[in] ProgName The name of the program, \c argv[0], used to print
+/// errors.
+/// \param[out] Style Structure describing the style reformatting should follow.
+/// \param[in] Diagnostics For error output.
+///
+/// \returns \li 0 if the command-line option is absent
+/// \li 1 if the command-line option is present and well formed.
+/// \li -1 if the command-line option is present but describes an
+/// unknown built-in style or the custom style description file
+/// couldn't be parsed.
+static int handleFormatStyle(const char *ProgName, format::FormatStyle &Style,
+ DiagnosticsEngine &Diagnostics) {
+ if (FormatStyleOpt.getNumOccurrences() <= 0)
+ return 0;
+
+ std::string OptValue = FormatStyleOpt;
+ if (OptValue.empty())
+ OptValue = "llvm";
+
+ if (!format::getPredefinedStyle(OptValue, &Style)) {
+ llvm::StringRef ConfigFilePath = FormatStyleOpt;
+ llvm::OwningPtr<llvm::MemoryBuffer> Text;
+ llvm::error_code ec;
+
+ ec = llvm::MemoryBuffer::getFile(ConfigFilePath, Text);
+ if (!ec)
+ ec = parseConfiguration(Text->getBuffer(), &Style);
+
+ if (ec) {
+ // FIXME: Use Diagnostics instead of llvm::errs().
+ llvm::errs() << ProgName << ": invalid format style " << FormatStyleOpt
+ << ": " << ec.message() << "\n";
+ return -1;
+ }
+ }
+
+ Style.Standard = clang::format::FormatStyle::LS_Auto;
+
+ return 1;
+}
+
// Helper object to remove the TUReplacement files (triggered by
// "remove-change-desc-files" command line option) when exiting current scope.
class ScopedFileRemover {
@@ -50,14 +106,69 @@
clang::DiagnosticsEngine &Diag;
};
+/// \brief Code changed by replacements in \c GroupedReplacements is reformatted
+/// according to \c Style.
+///
+/// \param[in] AppliedReplacements Rewriter with Replacements applied.
+/// \param[in] GroupedReplacements Replacements used for calculating changed
+/// ranges.
+/// \param[in] Diagnostics For diagnostics.
+/// \param[in] Style Formatting style to follow.
+/// \param[in,out] FormattingRewriter Assumed to be a Rewriter backed by a
+/// SourceManager where files can be overridden with new content from \c
+/// AppliedReplacements. Upon output, contains buffers containing formatted
+/// code.
+static void formatReplacements(const Rewriter &AppliedReplacements,
+ const FileToReplacementsMap &GroupedReplacements,
+ DiagnosticsEngine &Diagnostics,
+ format::FormatStyle Style,
+ Rewriter &FormattingRewriter) {
+ SourceManager &FormattingSM = FormattingRewriter.getSourceMgr();
+
+ // Update FormattingRewriter's SourceManager with new content from
+ // AppliedReplacements.
+ for (Rewriter::const_buffer_iterator
+ BufferI = AppliedReplacements.buffer_begin(),
+ BufferE = AppliedReplacements.buffer_end();
+ BufferI != BufferE; ++BufferI) {
+ const char *FileName = AppliedReplacements.getSourceMgr()
+ .getFileEntryForID(BufferI->first)
+ ->getName();
+
+ std::string NewData(BufferI->second.begin(), BufferI->second.end());
+ FormattingSM.overrideFileContents(
+ FormattingSM.getFileManager().getFile(FileName),
+ llvm::MemoryBuffer::getMemBufferCopy(NewData));
+
+ RangeVec Ranges;
+ calculateChangedRanges(GroupedReplacements.lookup(FileName), Ranges);
+
+ std::vector<clang::tooling::Replacement> FormattingReplacements;
+ reformatRanges(FileName, Ranges, FormattingSM, Style,
+ FormattingReplacements);
+
+ if (!tooling::applyAllReplacements(FormattingReplacements,
+ FormattingRewriter))
+ errs() << "Failed to apply reformatting replacements for " << FileName
+ << "\n";
+ }
+}
+
int main(int argc, char **argv) {
cl::ParseCommandLineOptions(argc, argv);
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
DiagnosticsEngine Diagnostics(
IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()),
DiagOpts.getPtr());
+ // Handle the -format-style option if present.
+ format::FormatStyle FormatStyle;
+ int FormatOptResult = handleFormatStyle(argv[0], FormatStyle, Diagnostics);
+ if (FormatOptResult < 0)
+ return 1;
+ bool DoFormat = FormatOptResult > 0;
+
TUReplacements TUs;
TUReplacementFiles TURFiles;
@@ -83,14 +194,53 @@
if (!mergeAndDeduplicate(TUs, GroupedReplacements, SM))
return 1;
- Rewriter DestRewriter(SM, LangOptions());
- if (!applyReplacements(GroupedReplacements, DestRewriter)) {
+ Rewriter ReplacementsRewriter(SM, LangOptions());
+
+ if (!applyReplacements(GroupedReplacements, ReplacementsRewriter)) {
errs() << "Failed to apply all replacements. No changes made.\n";
return 1;
}
- if (!writeFiles(DestRewriter))
- return 1;
+ // Separate File/SourceManager required since we can't update the content of
+ // a file in an existing SourceManager once it's been used by that
+ // SourceManager.
+ FileManager FormattingFiles((FileSystemOptions()));
+ SourceManager FormattingSM(Diagnostics, FormattingFiles);
+ Rewriter FormattingRewriter(FormattingSM, LangOptions());
+ if (DoFormat)
+ formatReplacements(ReplacementsRewriter, GroupedReplacements, Diagnostics,
+ FormatStyle, FormattingRewriter);
+
+ // Write modified file to disk.
+ for (Rewriter::const_buffer_iterator
+ BufferI = ReplacementsRewriter.buffer_begin(),
+ BufferE = ReplacementsRewriter.buffer_end();
+ BufferI != BufferE; ++BufferI) {
+ const char *FileName = SM.getFileEntryForID(BufferI->first)->getName();
+
+ std::string ErrorInfo;
+ llvm::raw_fd_ostream FileStream(FileName, ErrorInfo);
+ if (!ErrorInfo.empty()) {
+ llvm::errs() << "Could not open " << FileName << " for writing\n";
+ continue;
+ }
+
+ // If this file was formatted, write the formatted buffer to disk instead
+ // of the unformatted buffer. Note the use of FormattingFiles and
+ // FormattignSM here to get FID. This is necessary since these are the
+ // objects FormattingRewriter was constructed with.
+ const clang::FileEntry *Entry = FormattingFiles.getFile(FileName);
+ assert(Entry && "expected an existing file");
+ FileID ID = FormattingSM.translateFile(Entry);
+ const RewriteBuffer *FormattedBuffer = 0;
+ if (!ID.isInvalid())
+ FormattedBuffer = FormattingRewriter.getRewriteBufferFor(ID);
+
+ if (FormattedBuffer)
+ FormattedBuffer->write(FileStream);
+ else
+ BufferI->second.write(FileStream);
+ }
return 0;
}
Index: test/clang-apply-replacements/Inputs/format/no.cpp
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/Inputs/format/no.cpp
@@ -0,0 +1,6 @@
+class C {};
+
+void f() { // This comment necessary to prevent formatting as void f() { ... }
+ C *a = new C();
+ // CHECK: {{^\ \ auto\ a\ \=\ new\ C\(\);}}
+}
Index: test/clang-apply-replacements/Inputs/format/no.yaml
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/Inputs/format/no.yaml
@@ -0,0 +1,8 @@
+---
+MainSourceFile: no.cpp
+Replacements:
+ - FilePath: $(path)/no.cpp
+ Offset: 94
+ Length: 3
+ ReplacementText: 'auto '
+...
Index: test/clang-apply-replacements/Inputs/format/yes.cpp
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/Inputs/format/yes.cpp
@@ -0,0 +1,9 @@
+class MyType012345678901234567890123456789 {};
+
+void f() {
+ MyType012345678901234567890123456789 *a =
+ new MyType012345678901234567890123456789();
+ // CHECK: {{^\ \ auto\ a\ \=\ new\ MyType012345678901234567890123456789\(\);}}
+
+ delete a;
+}
Index: test/clang-apply-replacements/Inputs/format/yes.yaml
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/Inputs/format/yes.yaml
@@ -0,0 +1,8 @@
+---
+MainSourceFile: yes.cpp
+Replacements:
+ - FilePath: $(path)/yes.cpp
+ Offset: 61
+ Length: 38
+ ReplacementText: 'auto '
+...
Index: test/clang-apply-replacements/format.cpp
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/format.cpp
@@ -0,0 +1,13 @@
+// RUN: mkdir -p %T/Inputs/format
+//
+// yes.cpp requires formatting after replacements are applied. no.cpp does not.
+// The presence of no.cpp ensures that files that don't need formatting still
+// have their new state written to disk after applying replacements.
+//
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/format/yes.cpp > %T/Inputs/format/yes.cpp
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/format/no.cpp > %T/Inputs/format/no.cpp
+// RUN: sed "s#\$(path)#%/T/Inputs/format#" %S/Inputs/format/yes.yaml > %T/Inputs/format/yes.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/format#" %S/Inputs/format/no.yaml > %T/Inputs/format/no.yaml
+// RUN: clang-apply-replacements -format %T/Inputs/format
+// RUN: FileCheck --strict-whitespace -input-file=%T/Inputs/format/yes.cpp %S/Inputs/format/yes.cpp
+// RUN: FileCheck --strict-whitespace -input-file=%T/Inputs/format/no.cpp %S/Inputs/format/no.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
