Added Fixer implementation and fix() interface in clang-format for removing redundant code.

[ioeric]

Summary

After applying replacements, redundant code like extra commas or empty namespaces
might be introduced. Fixer can detect and remove any redundant code introduced by replacements.
The current implementation only handles redundant commas.

[ioeric]

include/clang/Format/Format.h

813 ↗

(On Diff #51905)

I am adding reformatting after fixing, and I am wondering if we can get rid of the Style parameter here (since the style does not really matter in code fixing, I think?) and let the reformat handling the style with the use case like:

FinalReplaces = formatReplacements(Code, fixReplacements(Code, Replaces), Style);

where fixReplacements is a function parallel to formatReplacements.

[ioeric]

• Added fixReplacements() that fix and reformat replacements.

[ioeric]

• Added fix for empty namespace.
• Merge multiple continuous token deletions into one big replacement; minor code styling.
• refactored code to reduce redundancy.

[djasper]

include/clang/Format/Format.h

770 ↗

(On Diff #52547)

I think we should not automatically format the replacements after fixing. This function can easily be combined with formatReplacements if that is desired.

802 ↗

(On Diff #52547)

This sentence doesn't make sense.

803 ↗

(On Diff #52547)

I believe the style will become important as certain aspects of fixing are going to be controlled via the style. E.g. some styles might want to turn:

if (a)
  return;

into:

if (a) {
  return;
}

lib/Format/Format.cpp

1446 ↗

(On Diff #52547)

I'd probably move all of these out into a class so you don't need to pass the same set of parameters between all of them and so that you can make those private that aren't meant to be called directly.

1596 ↗

(On Diff #52547)

I am not sure, this is the right class to pull out. It still has a lot of overlap with formatter. Maybe it is instead a better idea to pull out each fixer into its own class and pull them into the formatter.

1651 ↗

(On Diff #52547)

The Tokens parameter is unused!?

1654 ↗

(On Diff #52547)

I am not (yet) convinced that the fundamental design here makes sense. So far it seems to me that all fixers will either operate within a single line (ctor fixer and probably most others) or that they do something specific on multiple lines (namespace fixer and empty public/private section fixer and maybe others). Creating a new way to iterate over all tokens of all lines doesn't really add useful functionality here AFAICT.

1657 ↗

(On Diff #52547)

No braces. Here and at all other single-statement ifs.

1690 ↗

(On Diff #52547)

Having a function called "reset" that is only called once is weird. Just do this in the constructor?

1748 ↗

(On Diff #52547)

Comment on what this is doing.

1749 ↗

(On Diff #52547)

Ed? Either use E or End.

1891 ↗

(On Diff #52547)

Why is this needed?

1909 ↗

(On Diff #52547)

A lot of these need documentation. What's the current line? What is the last token (last token that was read? last token of line? last token of file?). What are RedundantTokens?

2356 ↗

(On Diff #52547)

Too much code duplication. Find a way to avoid that (passing in the calls to either reformat() or fix() as lambdas to a shared internal function).

unittests/Format/FormatTest.cpp

11236 ↗

(On Diff #52547)

All of this should go into a different file.

[ioeric]

• Refactored the code to reduce code duplication. Code styling. Moved test cases for Fixer into a new file FixTest.cpp.
• Added RangeManager to manage affected ranges. make empty namespace fixer delete tokens not in an affected line.

lib/Format/Format.cpp

1596 ↗

(On Diff #52547)

But I'm not quite sure if implementing the fixer in formatter works better since they look like two parallel classes to me. And it might make the formatter more complicated to use if we mix it with fixer. I think adding another layer of abstraction might be one way to reduce duplication?

1654 ↗

(On Diff #52547)

I've also considered the way formatter does, which iterates over lines. But it seems to be easier to write fixer, especially multi-line fixer, if we can just ignore lines. Working with lines seems to add another layer of complexity here?

[djasper]

lib/Format/Format.cpp

1654 ↗

(On Diff #52676)

That I don't understand. Almost all fixers (cleaning up commas, constructor initializers, etc.) will only ever look at a single line.

The matchers that do make multiline fixes (empty namespaces, empty public/private sections) only look at complete lines, i.e. only ever use AnnotatedLine::startsWith(...) and don't iterate over the tokens, AFAICT.

I really think that giving the option to iterate over all tokens of multiple lines does more harm than good. Among several other things, error recovery is made harder. It never makes sense for the ctor-initializer fixer to leave its line (or else you'll just ignore some nice error recovery already implemented in the UnwrappedLineParser).

1693 ↗

(On Diff #52676)

I have a comment here (and in some other places), but I think this will be different anyway if we move to line-based fixers. So I am holding off with those comments for now.

1718 ↗

(On Diff #52676)

Just go to CurrentToken->MatchingParen!?

1824 ↗

(On Diff #52676)

I am happy to show you how the implementation here gets much simpler if you only iterate over AnnotatedLines.

1916 ↗

(On Diff #52676)

I think "redundant" is the wrong word here. How about DeletedTokens?

[ioeric]

• Change implementation of fixer to iterate by lines. TODO: refactor Formatter and Fixer to reduce duplication.

[djasper]

include/clang/Format/Format.h

769 ↗

(On Diff #52832)

I am actually not sure that fixReplacements is the right terminology here (sorry that I haven't discovered this earlier). It isn't really fixing the replacements (they aren't necessarily broken). How about cleanupAroundReplacements? Manuel, what do you think?

lib/Format/Format.cpp

1596 ↗

(On Diff #52832)

But like this, it is way to much code duplication for me, both in number of lines of code as well as code complexity. Basically everything in fix() is duplicated along with most of the class members etc. The main difference seems to be that runFix() is called instead of format. There must be a way to have only one implementation of that code.

I think (but cannot see all the consequences yet) you should merge the two classes. And if that leads to too much complexity, you can probably pull out runFix and everything it calls into a separate class as well as possibly format() and everything that format calls.

1597 ↗

(On Diff #52832)

Why are you doing this?

1664 ↗

(On Diff #52832)

Can you move the ConstructorInitializer fixer and everything that belongs to it to a different patch? I want to look at that in more detail and two smaller patches make this easier for me.

1672 ↗

(On Diff #52832)

No need for this boolean. Write this as:

for (FormatToken *Tok = Line.First;; Tok = Tok->Next) {
  if (!Tok->is(tok::comment))
    return false;
}
return true;

And maybe we should (in a separate patch) add an iterator interface to AnnotatedLine so that we can use range-based for loops.

1695 ↗

(On Diff #52832)

Why not return -1 here?

1699 ↗

(On Diff #52832)

Move the startsWith into containsOnlyComments.

1701 ↗

(On Diff #52832)

I'd write this in a very different order:

if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace))
  break;
if (AnnotatedLines[CurrentLine]->startsWith(kw_namespaces) || ..) {
  int NewLine = checkEmptyNamespace(CurrentLine);
  if (NewLine == -1)
    return -1;
  continue;
if (!containsOnlyComments(..))
  return -1;

1702 ↗

(On Diff #52832)

It's a bit subtle that this works correctly. At least I needed to think quickly why you wouldn't falsely run into this in

namespace N {
void f() {
}
}

But of course, then the namespace wouldn't be empty. Please leave a comment explaining this briefly.

1717 ↗

(On Diff #52832)

Hm, this seems very inelegant. If you have:

namespace A {
namespace B {
namespace C {
}
}
}

"namespace C {" and "}" gets delete three times. At the very least you should add

if (AnnotatedLines[i]->Deleted)
  continue;

Thinking about this some more, it will get deleted even more often has even the outer loop in runFix will call checkEmptyNamespace again for the inner namespaces even though they have been deleted already. So inner namespaces will bei deleted O(N^2) times if N is the nesting depth. Lets get that down to 1 instead ;-)

1818 ↗

(On Diff #52832)

Can you explain why you are doing this?

2286 ↗

(On Diff #52832)

Don't define a typedef for something that is only used once. Also, this is an internal function, how about writing this as:

template <typename T>
static tooling::Replacements
processReplacements(StringRef Code, const tooling::Replacements &Replaces, T ProcessFunc,
                    const FormatStyle &Style) {

}

No need to spell out the function type (I think).

[ioeric]

• Moved constructor initializer fixer to a separate patch; pull runFixer and runFormat into separate classes, and merge common code in CodeProcessor class.

[ioeric]

• Fixed a potential bug in checkEmptyNamespace.

[ioeric]

lib/Format/Format.cpp

1590 ↗

(On Diff #53202)

I'm not quite sure if the name is accurate. Do you have a better name?

1597 ↗

(On Diff #52832)

I am not quite sure here actually. This was copied from Formatter. And since consumeUnwrappedLine requires at lease one element in UnwrappedLines, I figured this is necessary initialization?

1818 ↗

(On Diff #52832)

I want to reduce the number of replacements so that when we do reformat on the fixed code, there could be fewer changed code ranges considering that the current implementation of computeAffectedLines is not that efficient when we have many ranges.

2286 ↗

(On Diff #52832)

Wow, this is really helpful! Thanks!

[djasper]

include/clang/Format/Format.h

800 ↗

(On Diff #53202)

Same as above, "fix" is probably not the right word. "cleanup" seems somewhat better. Here and everywhere else in this patch :-/.

lib/Format/Format.cpp

1446 ↗

(On Diff #53202)

Maybe call this AffectedRangeManager? That is the class's very specific purpose.

1590 ↗

(On Diff #53202)

I think the difficulty of finding a name somewhat stems from the fact that it does multiple things at once. I think the main purpose is to encapsulate the "flattening" of the different #if/#else branches in successive runs.

Manuel, do you have an idea for a good name here?

1607 ↗

(On Diff #53202)

It feels like these two functions increase the coupling between Formatter/Fixer and CodeProcesser and aren't really necessary. Can you just inline them at the locations where they are called?

1669 ↗

(On Diff #53202)

Lets not have this access. Provide getters.

1908 ↗

(On Diff #53202)

I see why you are doing it this way, but let me propose something else:

How about iterating over all of the lines for each thing to cleanup (namespaces, ctor-intializers, etc.). I don't think it makes a significant difference in the runtime. Then you can just skip all the lines that you have already looked at (i.e. the returned int).

Among other things, you can probably remove the duplication of the AnnotatedLines[CurrentLine]->startsWith(..) and maybe even the AnnotatedLine::Deleted field.

lib/Format/FormatToken.h

283 ↗

(On Diff #53202)

Do you need this? Wouldn't it be enough to add them to the set of deleted tokens and mark them as Finalized?

[ioeric]

• Addressed comments.

[ioeric]

• removed unused fields in AnnotatedLine.

[ioeric]

• minor changes in checkEmptyNamespace.

[djasper]

lib/Format/Format.cpp

1677 ↗

(On Diff #53370)

I think FormatStyle in the Processor should be const and Formatter should make its own copy to modify it.

1926 ↗

(On Diff #53370)

Why are you introducing a return value that you aren't using? I'd just return the number of subsequent lines to skip.

Overall, I'd do this a bit differently:

• Let it check the namespace starting at CurrentLine
• Delete empty namespaces within
• If it is empty, delete the outer namespace
• Always return how many lines it has looked at

That way, you don't need to store DeletedLines and you don't ever look at the same line multiple times.

1927 ↗

(On Diff #53370)

What about

namespace A { // really cool namespace

?

1987 ↗

(On Diff #53370)

I believe that this doesn't work in many circumstances. Not sure I can easily construct a test case for the namespace cleanup, but generally, AnnotatedLines are not in a strict order. The last token of a line isn't necessarily next to the first token of the next line. This happens, e.g. for nested blocks and if preprocessor lines are intermingled with other lines.

1990–1993 ↗

(On Diff #53370)

I think this should hardly matter considering that we are probably only going to have a few instances of cleanup replacements and the range calculation is not that inefficient.

2224 ↗

(On Diff #53370)

Do we also need that if we spell out the ProcessFunc above? Maybe that's actually less painful compared to writing these lambdas?

[ioeric]

• Do not merge multiple lines when generate fixes. Added test cases with comments around namespace.
• Refactor: pull Annotator into Formatter/Cleaner from CodeProcessor. Moved AffectedRangeManager to a separate file.

[klimek]

include/clang/Format/Format.h

769 ↗

(On Diff #53866)

cleanupAroundReplacements sounds good.

lib/Format/Format.cpp

1466 ↗

(On Diff #53866)

Nit: I think it's idiomatic to call this a "Callback".

2143 ↗

(On Diff #53866)

This is unexpected: I'd not have expected the CodeProcessor to be used to keep state apart from what's used during a Process() run. I think we'll need a different name.

2176 ↗

(On Diff #53866)

The way the Processor is passed into the callbacks so they can call stuff on the processor again makes me think this is really tightly coupled and should use inheritance.

Alternatively, we might want to try to break up the CodeProcessor into a part that just runs the callback and keeps the state for running the callback around, and an interface that is used by the callbacks - I'll need to look more closely into this though.

[klimek]

lib/Format/Format.cpp

1450 ↗

(On Diff #53866)

After pondering a bit more, it seems like CodeProcessor is too generic: this is more something that runs analyses based on annotated tokens, so I'd call it TokenAnalyzer or something. That would also make it obvious to push some of the duplicated functionality from Formatter and Cleaner down (namely everything up to where we have generated the annotated tokens).

I'm still not completely convinced either way regarding inheritance vs. composition, but if we have a TokenAnalyzer, we'd also have a nice is-a relationship. While I generally agree to prefer composition, I also think we need to look out for where inheritance actually makes things clearer.

[ioeric]

• Rebased
• Make Formatter and Cleaner inherit from TokenAnalyzer (new name for CodeProcessor).

[ioeric]

• Added comments for endsWithInternal().

[ioeric]

PING.

[djasper]

lib/Format/Format.cpp

1544 ↗

(On Diff #54476)

Move this into the base class?

2097 ↗

(On Diff #54476)

Instead, create a class Environment that does all of these and makes SourceMgr, ID and CharRanges available via getters (maybe more). Then you should be able to just instantiate an environment and call the other reformat/cleanup function with the corresponding arguments.

[ioeric]

• Refactored - added Environment class.

[ioeric]

• Merged VirtualEnvironment into Environment.

[klimek]

We'll probably want Daniel to also take another look over it, as it's a pretty substantial change that will haunt us for a while, but I think this now pretty much looks like I expect it to look.

lib/Format/Format.cpp

1509 ↗

(On Diff #54837)

Don't name it SourceMgr, as there is a class of that name. Unfortunately we'll probably want to name it SM, like everywhere else in clang :(

1454 ↗

(On Diff #54848)

Won't deleting this line have the same effect?

1489–1490 ↗

(On Diff #54848)

There is no base class any more.

1872 ↗

(On Diff #54848)

In LLVM TODOs are spelled FIXME.

[ioeric]

• Addressed comments.

[ioeric]

@Daniel, sorry that I forgot to have you look at the final version before
submitting it...

[djasper]

cfe/trunk/lib/Format/AffectedRangeManager.h

59

And an empty line between functions and data members here.

66

Fix header guard comment

cfe/trunk/lib/Format/Format.cpp

1355

What happened here?

1463

Hand in CharRanges as ArrayRef or const vector&.

1471

Why don't you do this in the constructor? Seems asymmetric to just call a constructor in the one codepath but a factory function in the other one.

1519

Would it be sufficient to return a const SourceManager&? I guess not, but I'd like to understand where it breaks. If we could return a const source manager here, that would enable us to pass around a "const Environment&" at a few places.

[ioeric]

cfe/trunk/lib/Format/Format.cpp

1355

This line exceeded 80 characters and was formatted by clang-format when I ran clang-format across it...but I guess this change was out of the scope of this patch...sorry about that.

1471

We have a reference member "SourceManager &SM" that can only be initialized after setting up the environment...I didn't know if constructor would work in this case.

lib/Format/Format.cpp

1544 ↗

(On Diff #54476)

The way we calculate affected ranges are the same for Cleaner and Formatter. But in the future, we might want more accurate affected ranges for Cleaner.

cfe/trunk/include/clang/Format/Format.h

tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
                                    unsigned *Cursor = nullptr);
 
 /// \brief Returns the replacements corresponding to applying and formatting
 /// \p Replaces.
 tooling::Replacements formatReplacements(StringRef Code,
                                          const tooling::Replacements &Replaces,
                                          const FormatStyle &Style);
 
+/// \brief Returns the replacements corresponding to applying \p Replaces and
+/// cleaning up the code after that.
+tooling::Replacements
+cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
+                          const FormatStyle &Style);
+
 /// \brief Reformats the given \p Ranges in the file \p ID.
 ///
 /// Each range is extended on either end to its next bigger logic unit, i.e.
 /// everything that might influence its formatting or might be influenced by its
 /// formatting.
 ///
 /// Returns the ``Replacements`` necessary to make all \p Ranges comply with
 /// \p Style.

 /// \brief Reformats the given \p Ranges in \p Code.
 ///
 /// Otherwise identical to the reformat() function using a file ID.
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
                                ArrayRef<tooling::Range> Ranges,
                                StringRef FileName = "<stdin>",
                                bool *IncompleteFormat = nullptr);
 
+/// \brief Clean up any erroneous/redundant code in the given \p Ranges in the
+/// file \p ID.
+///
+/// Returns the ``Replacements`` that clean up all \p Ranges in the file \p ID.
+tooling::Replacements cleanup(const FormatStyle &Style,
+                              SourceManager &SourceMgr, FileID ID,
+                              ArrayRef<CharSourceRange> Ranges);
+
+/// \brief Clean up any erroneous/redundant code in the given \p Ranges in \p
+/// Code.
+///
+/// Otherwise identical to the cleanup() function using a file ID.
+tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code,
+                              ArrayRef<tooling::Range> Ranges,
+                              StringRef FileName = "<stdin>");
+
 /// \brief Returns the ``LangOpts`` that the formatter expects you to set.
 ///
 /// \param Style determines specific settings for lexing mode.
 LangOptions getFormattingLangOpts(const FormatStyle &Style = getLLVMStyle());
 
 /// \brief Description to be used for help text for a ``llvm::cl`` option for
 /// specifying format style. The description is closely related to the operation
 /// of ``getStyle()``.

cfe/trunk/lib/Format/AffectedRangeManager.h

+//===--- AffectedRangeManager.h - Format C++ code ---------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief AffectedRangeManager class manages affected ranges in the code.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_FORMAT_AFFECTEDRANGEMANAGER_H
+#define LLVM_CLANG_LIB_FORMAT_AFFECTEDRANGEMANAGER_H
+
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace format {
+
+struct FormatToken;
+class AnnotatedLine;
+
+class AffectedRangeManager {
+public:
+  AffectedRangeManager(SourceManager &SourceMgr,
+                       const ArrayRef<CharSourceRange> Ranges)
+      : SourceMgr(SourceMgr), Ranges(Ranges.begin(), Ranges.end()) {}
+
+  // Determines which lines are affected by the SourceRanges given as input.
+  // Returns \c true if at least one line between I and E or one of their
+  // children is affected.
+  bool computeAffectedLines(SmallVectorImpl<AnnotatedLine *>::iterator I,
+                            SmallVectorImpl<AnnotatedLine *>::iterator E);
+
+  // Returns true if 'Range' intersects with one of the input ranges.
+  bool affectsCharSourceRange(const CharSourceRange &Range);
+
+private:
+  // Returns true if the range from 'First' to 'Last' intersects with one of the
+  // input ranges.
+  bool affectsTokenRange(const FormatToken &First, const FormatToken &Last,
+                         bool IncludeLeadingNewlines);
+
+  // Returns true if one of the input ranges intersect the leading empty lines
+  // before 'Tok'.
+  bool affectsLeadingEmptyLines(const FormatToken &Tok);
+
+  // Marks all lines between I and E as well as all their children as affected.
+  void markAllAsAffected(SmallVectorImpl<AnnotatedLine *>::iterator I,
+                         SmallVectorImpl<AnnotatedLine *>::iterator E);
+
+  // Determines whether 'Line' is affected by the SourceRanges given as input.
+  // Returns \c true if line or one if its children is affected.
+  bool nonPPLineAffected(AnnotatedLine *Line,
+                         const AnnotatedLine *PreviousLine);
+  SourceManager &SourceMgr;
+  const SmallVector<CharSourceRange, 8> Ranges;
+};
+
+} // namespace format
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_FORMAT_WHITESPACEMANAGER_H

cfe/trunk/lib/Format/AffectedRangeManager.cpp

+//===--- AffectedRangeManager.cpp - Format C++ code -----------------------===//
+//
+//                     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 implements AffectRangeManager class.
+///
+//===----------------------------------------------------------------------===//
+
+#include "AffectedRangeManager.h"
+
+#include "FormatToken.h"
+#include "TokenAnnotator.h"
+
+namespace clang {
+namespace format {
+
+bool AffectedRangeManager::computeAffectedLines(
+    SmallVectorImpl<AnnotatedLine *>::iterator I,
+    SmallVectorImpl<AnnotatedLine *>::iterator E) {
+  bool SomeLineAffected = false;
+  const AnnotatedLine *PreviousLine = nullptr;
+  while (I != E) {
+    AnnotatedLine *Line = *I;
+    Line->LeadingEmptyLinesAffected = affectsLeadingEmptyLines(*Line->First);
+
+    // If a line is part of a preprocessor directive, it needs to be formatted
+    // if any token within the directive is affected.
+    if (Line->InPPDirective) {
+      FormatToken *Last = Line->Last;
+      SmallVectorImpl<AnnotatedLine *>::iterator PPEnd = I + 1;
+      while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline) {
+        Last = (*PPEnd)->Last;
+        ++PPEnd;
+      }
+
+      if (affectsTokenRange(*Line->First, *Last,
+                            /*IncludeLeadingNewlines=*/false)) {
+        SomeLineAffected = true;
+        markAllAsAffected(I, PPEnd);
+      }
+      I = PPEnd;
+      continue;
+    }
+
+    if (nonPPLineAffected(Line, PreviousLine))
+      SomeLineAffected = true;
+
+    PreviousLine = Line;
+    ++I;
+  }
+  return SomeLineAffected;
+}
+
+bool AffectedRangeManager::affectsCharSourceRange(
+    const CharSourceRange &Range) {
+  for (SmallVectorImpl<CharSourceRange>::const_iterator I = Ranges.begin(),
+                                                        E = Ranges.end();
+       I != E; ++I) {
+    if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) &&
+        !SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin()))
+      return true;
+  }
+  return false;
+}
+
+bool AffectedRangeManager::affectsTokenRange(const FormatToken &First,
+                                             const FormatToken &Last,
+                                             bool IncludeLeadingNewlines) {
+  SourceLocation Start = First.WhitespaceRange.getBegin();
+  if (!IncludeLeadingNewlines)
+    Start = Start.getLocWithOffset(First.LastNewlineOffset);
+  SourceLocation End = Last.getStartOfNonWhitespace();
+  End = End.getLocWithOffset(Last.TokenText.size());
+  CharSourceRange Range = CharSourceRange::getCharRange(Start, End);
+  return affectsCharSourceRange(Range);
+}
+
+bool AffectedRangeManager::affectsLeadingEmptyLines(const FormatToken &Tok) {
+  CharSourceRange EmptyLineRange = CharSourceRange::getCharRange(
+      Tok.WhitespaceRange.getBegin(),
+      Tok.WhitespaceRange.getBegin().getLocWithOffset(Tok.LastNewlineOffset));
+  return affectsCharSourceRange(EmptyLineRange);
+}
+
+void AffectedRangeManager::markAllAsAffected(
+    SmallVectorImpl<AnnotatedLine *>::iterator I,
+    SmallVectorImpl<AnnotatedLine *>::iterator E) {
+  while (I != E) {
+    (*I)->Affected = true;
+    markAllAsAffected((*I)->Children.begin(), (*I)->Children.end());
+    ++I;
+  }
+}
+
+bool AffectedRangeManager::nonPPLineAffected(
+    AnnotatedLine *Line, const AnnotatedLine *PreviousLine) {
+  bool SomeLineAffected = false;
+  Line->ChildrenAffected =
+      computeAffectedLines(Line->Children.begin(), Line->Children.end());
+  if (Line->ChildrenAffected)
+    SomeLineAffected = true;
+
+  // Stores whether one of the line's tokens is directly affected.
+  bool SomeTokenAffected = false;
+  // Stores whether we need to look at the leading newlines of the next token
+  // in order to determine whether it was affected.
+  bool IncludeLeadingNewlines = false;
+
+  // Stores whether the first child line of any of this line's tokens is
+  // affected.
+  bool SomeFirstChildAffected = false;
+
+  for (FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
+    // Determine whether 'Tok' was affected.
+    if (affectsTokenRange(*Tok, *Tok, IncludeLeadingNewlines))
+      SomeTokenAffected = true;
+
+    // Determine whether the first child of 'Tok' was affected.
+    if (!Tok->Children.empty() && Tok->Children.front()->Affected)
+      SomeFirstChildAffected = true;
+
+    IncludeLeadingNewlines = Tok->Children.empty();
+  }
+
+  // Was this line moved, i.e. has it previously been on the same line as an
+  // affected line?
+  bool LineMoved = PreviousLine && PreviousLine->Affected &&
+                   Line->First->NewlinesBefore == 0;
+
+  bool IsContinuedComment =
+      Line->First->is(tok::comment) && Line->First->Next == nullptr &&
+      Line->First->NewlinesBefore < 2 && PreviousLine &&
+      PreviousLine->Affected && PreviousLine->Last->is(tok::comment);
+
+  if (SomeTokenAffected || SomeFirstChildAffected || LineMoved ||
+      IsContinuedComment) {
+    Line->Affected = true;
+    SomeLineAffected = true;
+  }
+  return SomeLineAffected;
+}
+
+} // namespace format
+} // namespace clang

cfe/trunk/lib/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangFormat
+  AffectedRangeManager.cpp
   BreakableToken.cpp
   ContinuationIndenter.cpp
   Format.cpp
   FormatToken.cpp
   TokenAnnotator.cpp
   UnwrappedLineFormatter.cpp
   UnwrappedLineParser.cpp
   WhitespaceManager.cpp
 
   LINK_LIBS
   clangBasic
   clangLex
   clangToolingCore
   )

cfe/trunk/lib/Format/Format.cpp

 //===--- Format.cpp - Format C++ code -------------------------------------===//
 //
 //                     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 implements functions declared in Format.h. This will be
 /// split into separate files as we go.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "clang/Format/Format.h"
+#include "AffectedRangeManager.h"
 #include "ContinuationIndenter.h"
 #include "TokenAnnotator.h"
 #include "UnwrappedLineFormatter.h"
 #include "UnwrappedLineParser.h"
 #include "WhitespaceManager.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/YAMLTraits.h"
+#include <memory>
 #include <queue>
 #include <string>
 
 #define DEBUG_TYPE "format-formatter"
 
 using clang::format::FormatStyle;
 
 LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string)
std::string configurationAsText(const FormatStyle &Style) {
   Output << NonConstStyle;
   return Stream.str();
 }
 
 namespace {
 
 class FormatTokenLexer {
 public:
-  FormatTokenLexer(SourceManager &SourceMgr, FileID ID, FormatStyle &Style,
-                   encoding::Encoding Encoding)
+  FormatTokenLexer(SourceManager &SourceMgr, FileID ID,
+                   const FormatStyle &Style, encoding::Encoding Encoding)
       : FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false),
         LessStashed(false), Column(0), TrailingWhitespace(0),
         SourceMgr(SourceMgr), ID(ID), Style(Style),
         IdentTable(getFormattingLangOpts(Style)), Keywords(IdentTable),
         Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false),
         MacroBlockBeginRegex(Style.MacroBlockBegin),
         MacroBlockEndRegex(Style.MacroBlockEnd) {
     Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
if (FirstNewlinePos == StringRef::npos) {
       Column = FormatTok->LastLineColumnWidth;
     }
 
     if (Style.Language == FormatStyle::LK_Cpp) {
       if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
             Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
                 tok::pp_define) &&
           std::find(ForEachMacros.begin(), ForEachMacros.end(),
-                    FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
+                    FormatTok->Tok.getIdentifierInfo()) !=
+              ForEachMacros.end()) {
         FormatTok->Type = TT_ForEachMacro;
       } else if (FormatTok->is(tok::identifier)) {
         if (MacroBlockBeginRegex.match(Text)) {
           FormatTok->Type = TT_MacroBlockBegin;
         } else if (MacroBlockEndRegex.match(Text)) {
           FormatTok->Type = TT_MacroBlockEnd;
         }
       }
     }
 
     return FormatTok;
   }
 
   FormatToken *FormatTok;
   bool IsFirstToken;
   bool GreaterStashed, LessStashed;
   unsigned Column;
   unsigned TrailingWhitespace;
   std::unique_ptr<Lexer> Lex;
   SourceManager &SourceMgr;
   FileID ID;
-  FormatStyle &Style;
+  const FormatStyle &Style;
   IdentifierTable IdentTable;
   AdditionalKeywords Keywords;
   encoding::Encoding Encoding;
   llvm::SpecificBumpPtrAllocator<FormatToken> Allocator;
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector<FormatToken *, 16> Tokens;
   SmallVector<IdentifierInfo *, 8> ForEachMacros;
case FormatStyle::LK_JavaScript:
     return "JavaScript";
   case FormatStyle::LK_Proto:
     return "Proto";
   default:
     return "Unknown";
   }
 }
 
-class Formatter : public UnwrappedLineConsumer {
+class Environment {
 public:
-  Formatter(const FormatStyle &Style, SourceManager &SourceMgr, FileID ID,
+  Environment(const FormatStyle &Style, SourceManager &SM, FileID ID,
               ArrayRef<CharSourceRange> Ranges)
-      : Style(Style), ID(ID), SourceMgr(SourceMgr),
-        Whitespaces(SourceMgr, Style,
-                    inputUsesCRLF(SourceMgr.getBufferData(ID))),
-        Ranges(Ranges.begin(), Ranges.end()), UnwrappedLines(1),
-        Encoding(encoding::detectEncoding(SourceMgr.getBufferData(ID))) {
+      : Style(Style), ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM) {
+  }
+
+  Environment(const FormatStyle &Style, FileID ID,
+              std::unique_ptr<FileManager> FileMgr,
+              std::unique_ptr<SourceManager> VirtualSM,
+              std::unique_ptr<DiagnosticsEngine> Diagnostics,
+              std::vector<CharSourceRange> CharRanges)
+      : Style(Style), ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
+        SM(*VirtualSM), FileMgr(std::move(FileMgr)),
+        VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {}
+
+  // This sets up an virtual file system with file \p FileName containing \p
+  // Code.
+  static std::unique_ptr<Environment>
+  CreateVirtualEnvironment(const FormatStyle &Style, StringRef Code,
+                           StringRef FileName,
+                           ArrayRef<tooling::Range> Ranges) {
+    // This is referenced by `FileMgr` and will be released by `FileMgr` when it
+    // is deleted.
+    IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
+        new vfs::InMemoryFileSystem);
+    // This is passed to `SM` as reference, so the pointer has to be referenced
+    // in `Environment` so that `FileMgr` can out-live this function scope.
+    std::unique_ptr<FileManager> FileMgr(
+        new FileManager(FileSystemOptions(), InMemoryFileSystem));
+    // This is passed to `SM` as reference, so the pointer has to be referenced
+    // by `Environment` due to the same reason above.
+    std::unique_ptr<DiagnosticsEngine> Diagnostics(new DiagnosticsEngine(
+        IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
+        new DiagnosticOptions));
+    // This will be stored as reference, so the pointer has to be stored in
+    // due to the same reason above.
+    std::unique_ptr<SourceManager> VirtualSM(
+        new SourceManager(*Diagnostics, *FileMgr));
+    InMemoryFileSystem->addFile(
+        FileName, 0, llvm::MemoryBuffer::getMemBuffer(
+                         Code, FileName, /*RequiresNullTerminator=*/false));
+    FileID ID = VirtualSM->createFileID(
+        FileMgr->getFile(FileName), SourceLocation(), clang::SrcMgr::C_User);
+    assert(ID.isValid());
+    SourceLocation StartOfFile = VirtualSM->getLocForStartOfFile(ID);
+    std::vector<CharSourceRange> CharRanges;
+    for (const tooling::Range &Range : Ranges) {
+      SourceLocation Start = StartOfFile.getLocWithOffset(Range.getOffset());
+      SourceLocation End = Start.getLocWithOffset(Range.getLength());
+      CharRanges.push_back(CharSourceRange::getCharRange(Start, End));
+    }
+    return llvm::make_unique<Environment>(Style, ID, std::move(FileMgr),
+                                          std::move(VirtualSM),
+                                          std::move(Diagnostics), CharRanges);
+  }
+
+  FormatStyle &getFormatStyle() { return Style; }
+
+  const FormatStyle &getFormatStyle() const { return Style; }
+
+  FileID getFileID() const { return ID; }
+
+  StringRef getFileName() const { return FileName; }
+
+  ArrayRef<CharSourceRange> getCharRanges() const { return CharRanges; }
+
+  SourceManager &getSourceManager() { return SM; }
+
+private:
+  FormatStyle Style;
+  FileID ID;
+  StringRef FileName;
+  SmallVector<CharSourceRange, 8> CharRanges;
+  SourceManager &SM;
+
+  // The order of these fields are important - they should be in the same order
+  // as they are created in `CreateVirtualEnvironment` so that they can be
+  // deleted in the reverse order as they are created.
+  std::unique_ptr<FileManager> FileMgr;
+  std::unique_ptr<SourceManager> VirtualSM;
+  std::unique_ptr<DiagnosticsEngine> Diagnostics;
+};
+
+class TokenAnalyzer : public UnwrappedLineConsumer {
+public:
+  TokenAnalyzer(Environment &Env)
+      : Env(Env), AffectedRangeMgr(Env.getSourceManager(), Env.getCharRanges()),
+        UnwrappedLines(1),
+        Encoding(encoding::detectEncoding(
+            Env.getSourceManager().getBufferData(Env.getFileID()))) {
     DEBUG(llvm::dbgs() << "File encoding: "
                        << (Encoding == encoding::Encoding_UTF8 ? "UTF8"
                                                                : "unknown")
                        << "\n");
-    DEBUG(llvm::dbgs() << "Language: " << getLanguageName(Style.Language)
+    DEBUG(llvm::dbgs() << "Language: "
+                       << getLanguageName(Env.getFormatStyle().Language)
                        << "\n");
   }
 
-  tooling::Replacements format(bool *IncompleteFormat) {
+  tooling::Replacements process() {
     tooling::Replacements Result;
-    FormatTokenLexer Tokens(SourceMgr, ID, Style, Encoding);
+    FormatTokenLexer Tokens(Env.getSourceManager(), Env.getFileID(),
+                            Env.getFormatStyle(), Encoding);
 
-    UnwrappedLineParser Parser(Style, Tokens.getKeywords(), Tokens.lex(),
-                               *this);
+    UnwrappedLineParser Parser(Env.getFormatStyle(), Tokens.getKeywords(),
+                               Tokens.lex(), *this);
     Parser.parse();
     assert(UnwrappedLines.rbegin()->empty());
     for (unsigned Run = 0, RunE = UnwrappedLines.size(); Run + 1 != RunE;
          ++Run) {
       DEBUG(llvm::dbgs() << "Run " << Run << "...\n");
       SmallVector<AnnotatedLine *, 16> AnnotatedLines;
+
+      TokenAnnotator Annotator(Env.getFormatStyle(), Tokens.getKeywords());
       for (unsigned i = 0, e = UnwrappedLines[Run].size(); i != e; ++i) {
         AnnotatedLines.push_back(new AnnotatedLine(UnwrappedLines[Run][i]));
+        Annotator.annotate(*AnnotatedLines.back());
       }
+
       tooling::Replacements RunResult =
-          format(AnnotatedLines, Tokens, Result, IncompleteFormat);
+          analyze(Annotator, AnnotatedLines, Tokens, Result);
+
       DEBUG({
         llvm::dbgs() << "Replacements for run " << Run << ":\n";
         for (tooling::Replacements::iterator I = RunResult.begin(),
                                              E = RunResult.end();
              I != E; ++I) {
           llvm::dbgs() << I->toString() << "\n";
         }
       });
       for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
         delete AnnotatedLines[i];
       }
       Result.insert(RunResult.begin(), RunResult.end());
-      Whitespaces.reset();
     }
     return Result;
   }
 
-  tooling::Replacements format(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-                               FormatTokenLexer &Tokens,
-                               tooling::Replacements &Result,
-                               bool *IncompleteFormat) {
-    TokenAnnotator Annotator(Style, Tokens.getKeywords());
-    for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
-      Annotator.annotate(*AnnotatedLines[i]);
-    }
+protected:
+  virtual tooling::Replacements
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens, tooling::Replacements &Result) = 0;
+
+  void consumeUnwrappedLine(const UnwrappedLine &TheLine) override {
+    assert(!UnwrappedLines.empty());
+    UnwrappedLines.back().push_back(TheLine);
+  }
+
+  void finishRun() override {
+    UnwrappedLines.push_back(SmallVector<UnwrappedLine, 16>());
+
+  // Stores Style, FileID and SourceManager etc.
+  Environment &Env;
+  // AffectedRangeMgr stores ranges to be fixed.
+  AffectedRangeManager AffectedRangeMgr;
+  SmallVector<SmallVector<UnwrappedLine, 16>, 2> UnwrappedLines;
+  encoding::Encoding Encoding;
+};
+
+class Formatter : public TokenAnalyzer {
+public:
+  Formatter(Environment &Env, bool *IncompleteFormat)
+      : TokenAnalyzer(Env), IncompleteFormat(IncompleteFormat) {}
+
+  tooling::Replacements
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens, tooling::Replacements &Result) override {
     deriveLocalStyle(AnnotatedLines);
-    computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end());
-    if (Style.Language == FormatStyle::LK_JavaScript &&
-        Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
+                                          AnnotatedLines.end());
+
+    if (Env.getFormatStyle().Language == FormatStyle::LK_JavaScript &&
+        Env.getFormatStyle().JavaScriptQuotes != FormatStyle::JSQS_Leave)
       requoteJSStringLiteral(AnnotatedLines, Result);
 
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
     }
 
     Annotator.setCommentLineLevels(AnnotatedLines);
-    ContinuationIndenter Indenter(Style, Tokens.getKeywords(), SourceMgr,
-                                  Whitespaces, Encoding,
+
+    WhitespaceManager Whitespaces(
+        Env.getSourceManager(), Env.getFormatStyle(),
+        inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID())));
+    ContinuationIndenter Indenter(Env.getFormatStyle(), Tokens.getKeywords(),
+                                  Env.getSourceManager(), Whitespaces, Encoding,
                                   BinPackInconclusiveFunctions);
-    UnwrappedLineFormatter(&Indenter, &Whitespaces, Style, Tokens.getKeywords(),
-                           IncompleteFormat)
+    UnwrappedLineFormatter(&Indenter, &Whitespaces, Env.getFormatStyle(),
+                           Tokens.getKeywords(), IncompleteFormat)
         .format(AnnotatedLines);
     return Whitespaces.generateReplacements();
   }
 
 private:
-  // Determines which lines are affected by the SourceRanges given as input.
-  // Returns \c true if at least one line between I and E or one of their
-  // children is affected.
-  bool computeAffectedLines(SmallVectorImpl<AnnotatedLine *>::iterator I,
-                            SmallVectorImpl<AnnotatedLine *>::iterator E) {
-    bool SomeLineAffected = false;
-    const AnnotatedLine *PreviousLine = nullptr;
-    while (I != E) {
-      AnnotatedLine *Line = *I;
-      Line->LeadingEmptyLinesAffected = affectsLeadingEmptyLines(*Line->First);
-
-      // If a line is part of a preprocessor directive, it needs to be formatted
-      // if any token within the directive is affected.
-      if (Line->InPPDirective) {
-        FormatToken *Last = Line->Last;
-        SmallVectorImpl<AnnotatedLine *>::iterator PPEnd = I + 1;
-        while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline) {
-          Last = (*PPEnd)->Last;
-          ++PPEnd;
-        }
-
-        if (affectsTokenRange(*Line->First, *Last,
-                              /*IncludeLeadingNewlines=*/false)) {
-          SomeLineAffected = true;
-          markAllAsAffected(I, PPEnd);
-        }
-        I = PPEnd;
-        continue;
-      }
-
-      if (nonPPLineAffected(Line, PreviousLine))
-        SomeLineAffected = true;
-
-      PreviousLine = Line;
-      ++I;
-    }
-    return SomeLineAffected;
-  }
- 
   // If the last token is a double/single-quoted string literal, generates a
   // replacement with a single/double quoted string literal, re-escaping the
   // contents in the process.
   void requoteJSStringLiteral(SmallVectorImpl<AnnotatedLine *> &Lines,
-                                 tooling::Replacements &Result) {
     for (AnnotatedLine *Line : Lines) {
       requoteJSStringLiteral(Line->Children, Result);
       if (!Line->Affected)
         continue;
       for (FormatToken *FormatTok = Line->First; FormatTok;
            FormatTok = FormatTok->Next) {
         StringRef Input = FormatTok->TokenText;
         if (!FormatTok->isStringLiteral() ||
             // NB: testing for not starting with a double quote to avoid
             // breaking
             // `template strings`.
-            (Style.JavaScriptQuotes == FormatStyle::JSQS_Single &&
+            (Env.getFormatStyle().JavaScriptQuotes ==
+                 FormatStyle::JSQS_Single &&
              !Input.startswith("\"")) ||
-            (Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
+            (Env.getFormatStyle().JavaScriptQuotes ==
+                 FormatStyle::JSQS_Double &&
              !Input.startswith("\'")))
           continue;
 
         // Change start and end quote.
-        bool IsSingle = Style.JavaScriptQuotes == FormatStyle::JSQS_Single;
+        bool IsSingle =
+            Env.getFormatStyle().JavaScriptQuotes == FormatStyle::JSQS_Single;
         SourceLocation Start = FormatTok->Tok.getLocation();
         auto Replace = [&](SourceLocation Start, unsigned Length,
                            StringRef ReplacementText) {
-          Result.insert(
-              tooling::Replacement(SourceMgr, Start, Length, ReplacementText));
+          Result.insert(tooling::Replacement(Env.getSourceManager(), Start,
+                                             Length, ReplacementText));
         };
         Replace(Start, 1, IsSingle ? "'" : "\"");
         Replace(FormatTok->Tok.getEndLoc().getLocWithOffset(-1), 1,
                 IsSingle ? "'" : "\"");
 
         // Escape internal quotes.
         size_t ColumnWidth = FormatTok->TokenText.size();
         bool Escaped = false;
         for (size_t i = 1; i < Input.size() - 1; i++) {
           switch (Input[i]) {
-            case '\\':
-              if (!Escaped && i + 1 < Input.size() &&
-                  ((IsSingle && Input[i + 1] == '"') ||
-                   (!IsSingle && Input[i + 1] == '\''))) {
-                // Remove this \, it's escaping a " or ' that no longer needs
-                // escaping
-                ColumnWidth--;
-                Replace(Start.getLocWithOffset(i), 1, "");
-                continue;
-              }
-              Escaped = !Escaped;
-              break;
-            case '\"':
-            case '\'':
-              if (!Escaped && IsSingle == (Input[i] == '\'')) {
-                // Escape the quote.
-                Replace(Start.getLocWithOffset(i), 0, "\\");
-                ColumnWidth++;
-              }
-              Escaped = false;
-              break;
-            default:
-              Escaped = false;
-              break;
           }
         }
 
         // For formatting, count the number of non-escaped single quotes in them
         // and adjust ColumnWidth to take the added escapes into account.
-        // FIXME(martinprobst): this might conflict with code breaking a long string
-        // literal (which clang-format doesn't do, yet). For that to work, this code
-        // would have to modify TokenText directly.
+        // FIXME(martinprobst): this might conflict with code breaking a long
+        // string literal (which clang-format doesn't do, yet). For that to
+        // work, this code would have to modify TokenText directly.
         FormatTok->ColumnWidth = ColumnWidth;
       }
     }
   }
 
-
-  // Determines whether 'Line' is affected by the SourceRanges given as input.
-  // Returns \c true if line or one if its children is affected.
-  bool nonPPLineAffected(AnnotatedLine *Line,
-                         const AnnotatedLine *PreviousLine) {
-    bool SomeLineAffected = false;
-    Line->ChildrenAffected =
-        computeAffectedLines(Line->Children.begin(), Line->Children.end());
-    if (Line->ChildrenAffected)
-      SomeLineAffected = true;
-
-    // Stores whether one of the line's tokens is directly affected.
-    bool SomeTokenAffected = false;
-    // Stores whether we need to look at the leading newlines of the next token
-    // in order to determine whether it was affected.
-    bool IncludeLeadingNewlines = false;
-
-    // Stores whether the first child line of any of this line's tokens is
-    // affected.
-    bool SomeFirstChildAffected = false;
-
-    for (FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
-      // Determine whether 'Tok' was affected.
-      if (affectsTokenRange(*Tok, *Tok, IncludeLeadingNewlines))
-        SomeTokenAffected = true;
-
-      // Determine whether the first child of 'Tok' was affected.
-      if (!Tok->Children.empty() && Tok->Children.front()->Affected)
-        SomeFirstChildAffected = true;
-
-      IncludeLeadingNewlines = Tok->Children.empty();
-    }
-
-    // Was this line moved, i.e. has it previously been on the same line as an
-    // affected line?
-    bool LineMoved = PreviousLine && PreviousLine->Affected &&
-                     Line->First->NewlinesBefore == 0;
-
-    bool IsContinuedComment =
-        Line->First->is(tok::comment) && Line->First->Next == nullptr &&
-        Line->First->NewlinesBefore < 2 && PreviousLine &&
-        PreviousLine->Affected && PreviousLine->Last->is(tok::comment);
-
-    if (SomeTokenAffected || SomeFirstChildAffected || LineMoved ||
-        IsContinuedComment) {
-      Line->Affected = true;
-      SomeLineAffected = true;
-    }
-    return SomeLineAffected;
-  }
-
-  // Marks all lines between I and E as well as all their children as affected.
-  void markAllAsAffected(SmallVectorImpl<AnnotatedLine *>::iterator I,
-                         SmallVectorImpl<AnnotatedLine *>::iterator E) {
-    while (I != E) {
-      (*I)->Affected = true;
-      markAllAsAffected((*I)->Children.begin(), (*I)->Children.end());
-      ++I;
-    }
-  }
-
-  // Returns true if the range from 'First' to 'Last' intersects with one of the
-  // input ranges.
-  bool affectsTokenRange(const FormatToken &First, const FormatToken &Last,
-                         bool IncludeLeadingNewlines) {
-    SourceLocation Start = First.WhitespaceRange.getBegin();
-    if (!IncludeLeadingNewlines)
-      Start = Start.getLocWithOffset(First.LastNewlineOffset);
-    SourceLocation End = Last.getStartOfNonWhitespace();
-    End = End.getLocWithOffset(Last.TokenText.size());
-    CharSourceRange Range = CharSourceRange::getCharRange(Start, End);
-    return affectsCharSourceRange(Range);
-  }
-
-  // Returns true if one of the input ranges intersect the leading empty lines
-  // before 'Tok'.
-  bool affectsLeadingEmptyLines(const FormatToken &Tok) {
-    CharSourceRange EmptyLineRange = CharSourceRange::getCharRange(
-        Tok.WhitespaceRange.getBegin(),
-        Tok.WhitespaceRange.getBegin().getLocWithOffset(Tok.LastNewlineOffset));
-    return affectsCharSourceRange(EmptyLineRange);
-  }
-
-  // Returns true if 'Range' intersects with one of the input ranges.
-  bool affectsCharSourceRange(const CharSourceRange &Range) {
-    for (SmallVectorImpl<CharSourceRange>::const_iterator I = Ranges.begin(),
-                                                          E = Ranges.end();
-         I != E; ++I) {
-      if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) &&
-          !SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin()))
-        return true;
-    }
-    return false;
-  }
-
   static bool inputUsesCRLF(StringRef Text) {
     return Text.count('\r') * 2 > Text.count('\n');
   }
 
   bool
   hasCpp03IncompatibleFormat(const SmallVectorImpl<AnnotatedLine *> &Lines) {
-    for (const AnnotatedLine* Line : Lines) {
+    for (const AnnotatedLine *Line : Lines) {
       if (hasCpp03IncompatibleFormat(Line->Children))
         return true;
       for (FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next) {
         if (Tok->WhitespaceRange.getBegin() == Tok->WhitespaceRange.getEnd()) {
           if (Tok->is(tok::coloncolon) && Tok->Previous->is(TT_TemplateOpener))
             return true;
           if (Tok->is(TT_TemplateCloser) &&
               Tok->Previous->is(TT_TemplateCloser))
             return true;
         }
       }
     }
     return false;
   }
 
   int countVariableAlignments(const SmallVectorImpl<AnnotatedLine *> &Lines) {
     int AlignmentDiff = 0;
-    for (const AnnotatedLine* Line : Lines) {
+    for (const AnnotatedLine *Line : Lines) {
       AlignmentDiff += countVariableAlignments(Line->Children);
       for (FormatToken *Tok = Line->First; Tok && Tok->Next; Tok = Tok->Next) {
         if (!Tok->is(TT_PointerOrReference))
           continue;
         bool SpaceBefore =
             Tok->WhitespaceRange.getBegin() != Tok->WhitespaceRange.getEnd();
         bool SpaceAfter = Tok->Next->WhitespaceRange.getBegin() !=
                           Tok->Next->WhitespaceRange.getEnd();
for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
         if (Tok->PackingKind == PPK_BinPacked)
           HasBinPackedFunction = true;
         if (Tok->PackingKind == PPK_OnePerLine)
           HasOnePerLineFunction = true;
 
         Tok = Tok->Next;
       }
     }
-    if (Style.DerivePointerAlignment)
-      Style.PointerAlignment = countVariableAlignments(AnnotatedLines) <= 0
-                                   ? FormatStyle::PAS_Left
+    if (Env.getFormatStyle().DerivePointerAlignment)
+      Env.getFormatStyle().PointerAlignment =
+          countVariableAlignments(AnnotatedLines) <= 0 ? FormatStyle::PAS_Left
                                                        : FormatStyle::PAS_Right;
-    if (Style.Standard == FormatStyle::LS_Auto)
-      Style.Standard = hasCpp03IncompatibleFormat(AnnotatedLines)
+    if (Env.getFormatStyle().Standard == FormatStyle::LS_Auto)
+      Env.getFormatStyle().Standard = hasCpp03IncompatibleFormat(AnnotatedLines)
                                           ? FormatStyle::LS_Cpp11
                                           : FormatStyle::LS_Cpp03;
     BinPackInconclusiveFunctions =
         HasBinPackedFunction || !HasOnePerLineFunction;
   }
 
-  void consumeUnwrappedLine(const UnwrappedLine &TheLine) override {
-    assert(!UnwrappedLines.empty());
-    UnwrappedLines.back().push_back(TheLine);
+  bool BinPackInconclusiveFunctions;
+  bool *IncompleteFormat;
+};
+
+// This class clean up the erroneous/redundant code around the given ranges in
+// file.
+class Cleaner : public TokenAnalyzer {
+public:
+  Cleaner(Environment &Env)
+      : TokenAnalyzer(Env),
+        DeletedTokens(FormatTokenLess(Env.getSourceManager())) {}
+
+  // FIXME: eliminate unused parameters.
+  tooling::Replacements
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens, tooling::Replacements &Result) override {
+    // FIXME: in the current implementation the granularity of affected range
+    // is an annotated line. However, this is not sufficient. Furthermore,
+    // redundant code introduced by replacements does not necessarily
+    // intercept with ranges of replacements that result in the redundancy.
+    // To determine if some redundant code is actually introduced by
+    // replacements(e.g. deletions), we need to come up with a more
+    // sophisticated way of computing affected ranges.
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
+                                          AnnotatedLines.end());
+
+    checkEmptyNamespace(AnnotatedLines);
+
+    return generateFixes();
   }
 
-  void finishRun() override {
-    UnwrappedLines.push_back(SmallVector<UnwrappedLine, 16>());
+private:
+  bool containsOnlyComments(const AnnotatedLine &Line) {
+    for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
+      if (Tok->isNot(tok::comment))
+        return false;
+    }
+    return true;
   }
 
-  FormatStyle Style;
-  FileID ID;
-  SourceManager &SourceMgr;
-  WhitespaceManager Whitespaces;
-  SmallVector<CharSourceRange, 8> Ranges;
-  SmallVector<SmallVector<UnwrappedLine, 16>, 2> UnwrappedLines;
+  // Iterate through all lines and remove any empty (nested) namespaces.
+  void checkEmptyNamespace(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
+    for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
+      auto &Line = *AnnotatedLines[i];
+      if (Line.startsWith(tok::kw_namespace) ||
+          Line.startsWith(tok::kw_inline, tok::kw_namespace)) {
+        checkEmptyNamespace(AnnotatedLines, i, i);
+      }
+    }
 
-  encoding::Encoding Encoding;
-  bool BinPackInconclusiveFunctions;
+    for (auto Line : DeletedLines) {
+      FormatToken *Tok = AnnotatedLines[Line]->First;
+      while (Tok) {
+        deleteToken(Tok);
+        Tok = Tok->Next;
+      }
+    }
+  }
+
+  // The function checks if the namespace, which starts from \p CurrentLine, and
+  // its nested namespaces are empty and delete them if they are empty. It also
+  // sets \p NewLine to the last line checked.
+  // Returns true if the current namespace is empty.
+  bool checkEmptyNamespace(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+                           unsigned CurrentLine, unsigned &NewLine) {
+    unsigned InitLine = CurrentLine, End = AnnotatedLines.size();
+    if (Env.getFormatStyle().BraceWrapping.AfterNamespace) {
+      // If the left brace is in a new line, we should consume it first so that
+      // it does not make the namespace non-empty.
+      // FIXME: error handling if there is no left brace.
+      if (!AnnotatedLines[++CurrentLine]->startsWith(tok::l_brace)) {
+        NewLine = CurrentLine;
+        return false;
+      }
+    } else if (!AnnotatedLines[CurrentLine]->endsWith(tok::l_brace)) {
+      return false;
+    }
+    while (++CurrentLine < End) {
+      if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace))
+        break;
+
+      if (AnnotatedLines[CurrentLine]->startsWith(tok::kw_namespace) ||
+          AnnotatedLines[CurrentLine]->startsWith(tok::kw_inline,
+                                                  tok::kw_namespace)) {
+        if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine))
+          return false;
+        CurrentLine = NewLine;
+        continue;
+      }
+
+      if (containsOnlyComments(*AnnotatedLines[CurrentLine]))
+        continue;
+
+      // If there is anything other than comments or nested namespaces in the
+      // current namespace, the namespace cannot be empty.
+      NewLine = CurrentLine;
+      return false;
+    }
+
+    NewLine = CurrentLine;
+    if (CurrentLine >= End)
+      return false;
+
+    // Check if the empty namespace is actually affected by changed ranges.
+    if (!AffectedRangeMgr.affectsCharSourceRange(CharSourceRange::getCharRange(
+            AnnotatedLines[InitLine]->First->Tok.getLocation(),
+            AnnotatedLines[CurrentLine]->Last->Tok.getEndLoc())))
+      return false;
+
+    for (unsigned i = InitLine; i <= CurrentLine; ++i) {
+      DeletedLines.insert(i);
+    }
+
+    return true;
+  }
+
+  // Delete the given token.
+  inline void deleteToken(FormatToken *Tok) {
+    if (Tok)
+      DeletedTokens.insert(Tok);
+  }
+
+  tooling::Replacements generateFixes() {
+    tooling::Replacements Fixes;
+    std::vector<FormatToken *> Tokens;
+    std::copy(DeletedTokens.begin(), DeletedTokens.end(),
+              std::back_inserter(Tokens));
+
+    // Merge multiple continuous token deletions into one big deletion so that
+    // the number of replacements can be reduced. This makes computing affected
+    // ranges more efficient when we run reformat on the changed code.
+    unsigned Idx = 0;
+    while (Idx < Tokens.size()) {
+      unsigned St = Idx, End = Idx;
+      while ((End + 1) < Tokens.size() &&
+             Tokens[End]->Next == Tokens[End + 1]) {
+        End++;
+      }
+      auto SR = CharSourceRange::getCharRange(Tokens[St]->Tok.getLocation(),
+                                              Tokens[End]->Tok.getEndLoc());
+      Fixes.insert(tooling::Replacement(Env.getSourceManager(), SR, ""));
+      Idx = End + 1;
+    }
+
+    return Fixes;
+  }
+
+  // Class for less-than inequality comparason for the set `RedundantTokens`.
+  // We store tokens in the order they appear in the translation unit so that
+  // we do not need to sort them in `generateFixes()`.
+  struct FormatTokenLess {
+    FormatTokenLess(SourceManager &SM) : SM(SM) {}
+
+    bool operator()(const FormatToken *LHS, const FormatToken *RHS) {
+      return SM.isBeforeInTranslationUnit(LHS->Tok.getLocation(),
+                                          RHS->Tok.getLocation());
+    }
+    SourceManager &SM;
+  };
+
+  // Tokens to be deleted.
+  std::set<FormatToken *, FormatTokenLess> DeletedTokens;
+  // The line numbers of lines to be deleted.
+  std::set<unsigned> DeletedLines;
 };
 
 struct IncludeDirective {
   StringRef Filename;
   StringRef Text;
   unsigned Offset;
   int Category;
 };
if (Pos == StringRef::npos || Pos + 1 == Code.size())
       break;
     SearchFrom = Pos + 1;
   }
   if (!IncludesInBlock.empty())
     sortIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces, Cursor);
   return Replaces;
 }
 
-tooling::Replacements formatReplacements(StringRef Code,
-                                         const tooling::Replacements &Replaces,
-                                         const FormatStyle &Style) {
+template <typename T>
+static tooling::Replacements
+processReplacements(T ProcessFunc, StringRef Code,
   if (Replaces.empty())
     return tooling::Replacements();
 
   std::string NewCode = applyAllReplacements(Code, Replaces);
   std::vector<tooling::Range> ChangedRanges =
       tooling::calculateChangedRanges(Replaces);
   StringRef FileName = Replaces.begin()->getFilePath();
+
   tooling::Replacements FormatReplaces =
-      reformat(Style, NewCode, ChangedRanges, FileName);
+      ProcessFunc(Style, NewCode, ChangedRanges, FileName);
+
+  return mergeReplacements(Replaces, FormatReplaces);
+}
 
-  tooling::Replacements MergedReplacements =
-      mergeReplacements(Replaces, FormatReplaces);
+tooling::Replacements formatReplacements(StringRef Code,
+                                         const tooling::Replacements &Replaces,
+                                         const FormatStyle &Style) {
+  // We need to use lambda function here since there are two versions of
+  // `reformat`.
+  auto Reformat = [](const FormatStyle &Style, StringRef Code,
+                     std::vector<tooling::Range> Ranges,
+                     StringRef FileName) -> tooling::Replacements {
+    return reformat(Style, Code, Ranges, FileName);
+  };
+  return processReplacements(Reformat, Code, Replaces, Style);
+}
 
-  return MergedReplacements;
+tooling::Replacements
+cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
+                          const FormatStyle &Style) {
+  // We need to use lambda function here since there are two versions of
+  // `cleanup`.
+  auto Cleanup = [](const FormatStyle &Style, StringRef Code,
+                    std::vector<tooling::Range> Ranges,
+                    StringRef FileName) -> tooling::Replacements {
+    return cleanup(Style, Code, Ranges, FileName);
+  };
+  return processReplacements(Cleanup, Code, Replaces, Style);
 }
 
-tooling::Replacements reformat(const FormatStyle &Style,
-                               SourceManager &SourceMgr, FileID ID,
-                               ArrayRef<CharSourceRange> Ranges,
+tooling::Replacements reformat(const FormatStyle &Style, SourceManager &SM,
+                               FileID ID, ArrayRef<CharSourceRange> Ranges,
                                bool *IncompleteFormat) {
   FormatStyle Expanded = expandPresets(Style);
   if (Expanded.DisableFormat)
     return tooling::Replacements();
-  Formatter formatter(Expanded, SourceMgr, ID, Ranges);
-  return formatter.format(IncompleteFormat);
+
+  Environment Env(Expanded, SM, ID, Ranges);
+  Formatter Format(Env, IncompleteFormat);
+  return Format.process();
 }
 
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
                                ArrayRef<tooling::Range> Ranges,
                                StringRef FileName, bool *IncompleteFormat) {
-  if (Style.DisableFormat)
+  FormatStyle Expanded = expandPresets(Style);
+  if (Expanded.DisableFormat)
     return tooling::Replacements();
 
-  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
-      new vfs::InMemoryFileSystem);
-  FileManager Files(FileSystemOptions(), InMemoryFileSystem);
-  DiagnosticsEngine Diagnostics(
-      IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
-      new DiagnosticOptions);
-  SourceManager SourceMgr(Diagnostics, Files);
-  InMemoryFileSystem->addFile(
-      FileName, 0, llvm::MemoryBuffer::getMemBuffer(
-                       Code, FileName, /*RequiresNullTerminator=*/false));
-  FileID ID = SourceMgr.createFileID(Files.getFile(FileName), SourceLocation(),
-                                     clang::SrcMgr::C_User);
-  SourceLocation StartOfFile = SourceMgr.getLocForStartOfFile(ID);
-  std::vector<CharSourceRange> CharRanges;
-  for (const tooling::Range &Range : Ranges) {
-    SourceLocation Start = StartOfFile.getLocWithOffset(Range.getOffset());
-    SourceLocation End = Start.getLocWithOffset(Range.getLength());
-    CharRanges.push_back(CharSourceRange::getCharRange(Start, End));
-  }
-  return reformat(Style, SourceMgr, ID, CharRanges, IncompleteFormat);
+  std::unique_ptr<Environment> Env =
+      Environment::CreateVirtualEnvironment(Expanded, Code, FileName, Ranges);
+  Formatter Format(*Env, IncompleteFormat);
+  return Format.process();
+
+tooling::Replacements cleanup(const FormatStyle &Style, SourceManager &SM,
+                              FileID ID, ArrayRef<CharSourceRange> Ranges) {
+  Environment Env(Style, SM, ID, Ranges);
+  Cleaner Clean(Env);
+  return Clean.process();
+}
+
+tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code,
+                              ArrayRef<tooling::Range> Ranges,
+                              StringRef FileName) {
+  std::unique_ptr<Environment> Env =
+      Environment::CreateVirtualEnvironment(Style, Code, FileName, Ranges);
+  Cleaner Clean(*Env);
+  return Clean.process();
 }
 
 LangOptions getFormattingLangOpts(const FormatStyle &Style) {
   LangOptions LangOpts;
   LangOpts.CPlusPlus = 1;
   LangOpts.CPlusPlus11 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
   LangOpts.CPlusPlus14 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
   LangOpts.LineComment = 1;
   bool AlternativeOperators = Style.Language == FormatStyle::LK_Cpp;
   LangOpts.CXXOperatorNames = AlternativeOperators ? 1 : 0;
   LangOpts.Bool = 1;
   LangOpts.ObjC1 = 1;
   LangOpts.ObjC2 = 1;
-  LangOpts.MicrosoftExt = 1; // To get kw___try, kw___finally.
+  LangOpts.MicrosoftExt = 1;    // To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
   return LangOpts;
 }
 
 const char *StyleOptionHelpDescription =
     "Coding style, currently supports:\n"
     "  LLVM, Google, Chromium, Mozilla, WebKit.\n"
     "Use -style=file to load style configuration from\n"

cfe/trunk/lib/Format/TokenAnnotator.h

public:
   }
 
   /// \c true if this line starts with the given tokens in order, ignoring
   /// comments.
   template <typename... Ts> bool startsWith(Ts... Tokens) const {
     return startsWithInternal(First, Tokens...);
   }
 
+  /// \c true if this line ends with the given tokens in reversed order,
+  /// ignoring comments.
+  /// For example, given tokens [T1, T2, T3, ...], the function returns true if
+  /// this line is like "... T3 T2 T1".
+  template <typename... Ts> bool endsWith(Ts... Tokens) const {
+    return endsWithInternal(Last, Tokens...);
+  }
+
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
   bool mightBeFunctionDefinition() const {
     assert(MightBeFunctionDecl);
     // FIXME: Line.Last points to other characters than tok::semi
     // and tok::lbrace.
     return !Last->isOneOf(tok::semi, tok::comment);
   }
bool startsWithInternal(const FormatToken *Tok, A K1, Ts... Tokens) const {
     // Skip comments before calling `startsWithInternal(Tok, K1)` so that  the
     // second call to `startsWithInternal` takes the correct `Tok->Next`, which
     // should be the next token of the token checked in the first call.
     while (Tok && Tok->is(tok::comment))
       Tok = Tok->Next;
     return Tok && startsWithInternal(Tok, K1) &&
            startsWithInternal(Tok->Next, Tokens...);
   }
+
+  template <typename A, typename... Ts>
+  bool endsWithInternal(const FormatToken *Tok, A K1) const {
+    // See the comments above in `startsWithInternal(Tok, K1)`.
+    while (Tok && Tok->is(tok::comment))
+      Tok = Tok->Previous;
+    return Tok && Tok->is(K1);
+  }
+
+  template <typename A, typename... Ts>
+  bool endsWithInternal(const FormatToken *Tok, A K1, Ts... Tokens) const {
+    // See the comments above in `startsWithInternal(Tok, K1, Tokens)`.
+    while (Tok && Tok->is(tok::comment))
+      Tok = Tok->Previous;
+    return Tok && endsWithInternal(Tok, K1) &&
+           endsWithInternal(Tok->Previous, Tokens...);
+  }
 };
 
 /// \brief Determines extra information about the tokens comprising an
 /// \c UnwrappedLine.
 class TokenAnnotator {
 public:
   TokenAnnotator(const FormatStyle &Style, const AdditionalKeywords &Keywords)
       : Style(Style), Keywords(Keywords) {}

cfe/trunk/unittests/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(FormatTests
+  CleanupTest.cpp
   FormatTest.cpp
   FormatTestJava.cpp
   FormatTestJS.cpp
   FormatTestProto.cpp
   FormatTestSelective.cpp
   SortIncludesTest.cpp
   )
 
 target_link_libraries(FormatTests
   clangBasic
   clangFormat
   clangFrontend
   clangRewrite
   clangToolingCore
   )

cfe/trunk/unittests/Format/CleanupTest.cpp

+//===- unittest/Format/CleanupTest.cpp - Code cleanup unit tests ----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Format/Format.h"
+
+#include "clang/Tooling/Core/Replacement.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace format {
+namespace {
+
+class CleanupTest : public ::testing::Test {
+protected:
+  std::string cleanup(llvm::StringRef Code,
+                      const std::vector<tooling::Range> &Ranges,
+                      const FormatStyle &Style = getLLVMStyle()) {
+    tooling::Replacements Replaces = format::cleanup(Style, Code, Ranges);
+
+    std::string Result = applyAllReplacements(Code, Replaces);
+    EXPECT_NE("", Result);
+    return Result;
+  }
+};
+
+TEST_F(CleanupTest, DeleteEmptyNamespaces) {
+  std::string Code = "namespace A {\n"
+                     "namespace B {\n"
+                     "} // namespace B\n"
+                     "} // namespace A\n\n"
+                     "namespace C {\n"
+                     "namespace D { int i; }\n"
+                     "inline namespace E { namespace { } }\n"
+                     "}";
+  std::string Expected = "\n\n\n\n\nnamespace C {\n"
+                         "namespace D { int i; }\n   \n"
+                         "}";
+  std::vector<tooling::Range> Ranges;
+  Ranges.push_back(tooling::Range(28, 0));
+  Ranges.push_back(tooling::Range(91, 6));
+  Ranges.push_back(tooling::Range(132, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, NamespaceWithSyntaxError) {
+  std::string Code = "namespace A {\n"
+                     "namespace B {\n" // missing r_brace
+                     "} // namespace A\n\n"
+                     "namespace C {\n"
+                     "namespace D int i; }\n"
+                     "inline namespace E { namespace { } }\n"
+                     "}";
+  std::string Expected = "namespace A {\n"
+                         "\n\n\nnamespace C {\n"
+                         "namespace D int i; }\n   \n"
+                         "}";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, EmptyNamespaceNotAffected) {
+  std::string Code = "namespace A {\n\n"
+                     "namespace {\n\n}}";
+  // Even though the namespaces are empty, but the inner most empty namespace
+  // block is not affected by the changed ranges.
+  std::string Expected = "namespace A {\n\n"
+                         "namespace {\n\n}}";
+  // Set the changed range to be the second "\n".
+  std::vector<tooling::Range> Ranges(1, tooling::Range(14, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, EmptyNamespaceWithCommentsNoBreakBeforeBrace) {
+  std::string Code = "namespace A {\n"
+                     "namespace B {\n"
+                     "// Yo\n"
+                     "} // namespace B\n"
+                     "} // namespace A\n"
+                     "namespace C { // Yo\n"
+                     "}";
+  std::string Expected = "\n\n\n\n\n\n";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, EmptyNamespaceWithCommentsBreakBeforeBrace) {
+  std::string Code = "namespace A\n"
+                     "/* Yo */ {\n"
+                     "namespace B\n"
+                     "{\n"
+                     "// Yo\n"
+                     "} // namespace B\n"
+                     "} // namespace A\n"
+                     "namespace C\n"
+                     "{ // Yo\n"
+                     "}\n";
+  std::string Expected = "\n\n\n\n\n\n\n\n\n\n";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  FormatStyle Style = getLLVMStyle();
+  Style.BraceWrapping.AfterNamespace = true;
+  std::string Result = cleanup(Code, Ranges, Style);
+  EXPECT_EQ(Expected, Result);
+}
+
+} // end namespace
+} // end namespace format
+} // end namespace clang

cfe/trunk/unittests/Format/FormatTest.cpp

Replaces.insert(tooling::Replacement(
       Context.Sources, Context.getLocation(ID, 4, 13), 1, "nullptr"));
 
   format::FormatStyle Style = format::getLLVMStyle();
   Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
   EXPECT_EQ(Expected, applyAllReplacements(
                           Code, formatReplacements(Code, Replaces, Style)));
 }
 
+TEST_F(ReplacementTest, FixOnlyAffectedCodeAfterReplacements) {
+  std::string Code = "namespace A {\n"
+                     "namespace B {\n"
+                     "  int x;\n"
+                     "} // namespace B\n"
+                     "} // namespace A\n"
+                     "\n"
+                     "namespace C {\n"
+                     "namespace D { int i; }\n"
+                     "inline namespace E { namespace { int y; } }\n"
+                     "int x=     0;"
+                     "}";
+  std::string Expected = "\n\nnamespace C {\n"
+                         "namespace D { int i; }\n\n"
+                         "int x=     0;"
+                         "}";
+  FileID ID = Context.createInMemoryFile("fix.cpp", Code);
+  tooling::Replacements Replaces;
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID, 3, 3), 6, ""));
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID, 9, 34), 6, ""));
+
+  format::FormatStyle Style = format::getLLVMStyle();
+  auto FinalReplaces = formatReplacements(
+      Code, cleanupAroundReplacements(Code, Replaces, Style), Style);
+  EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang