[PATCH] D18551: Added Fixer implementation and fix() interface in clang-format for removing redundant code.

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: cfe/trunk/lib/Format/AffectedRangeManager.h:59
@@ +58,3 @@
+                         const AnnotatedLine *PreviousLine);
+  SourceManager &SourceMgr;
+  const SmallVector<CharSourceRange, 8> Ranges;
----------------
And an empty line between functions and data members here.

================
Comment at: cfe/trunk/lib/Format/AffectedRangeManager.h:66
@@ +65,2 @@
+
+#endif // LLVM_CLANG_LIB_FORMAT_WHITESPACEMANAGER_H
----------------
Fix header guard comment

================
Comment at: cfe/trunk/lib/Format/Format.cpp:1355
@@ -1352,2 +1354,3 @@
           std::find(ForEachMacros.begin(), ForEachMacros.end(),
-                    FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
+                    FormatTok->Tok.getIdentifierInfo()) !=
+              ForEachMacros.end()) {
----------------
What happened here?

================
Comment at: cfe/trunk/lib/Format/Format.cpp:1463
@@ +1462,3 @@
+              std::unique_ptr<DiagnosticsEngine> Diagnostics,
+              std::vector<CharSourceRange> CharRanges)
+      : Style(Style), ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
----------------
Hand in CharRanges as ArrayRef or const vector&.

================
Comment at: cfe/trunk/lib/Format/Format.cpp:1471
@@ +1470,3 @@
+  static std::unique_ptr<Environment>
+  CreateVirtualEnvironment(const FormatStyle &Style, StringRef Code,
+                           StringRef FileName,
----------------
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.

================
Comment at: cfe/trunk/lib/Format/Format.cpp:1519
@@ +1518,3 @@
+
+  SourceManager &getSourceManager() { return SM; }
+
----------------
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.


Repository:
  rL LLVM

http://reviews.llvm.org/D18551