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

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 27 06:02:43 PDT 2016


ioeric marked 6 inline comments as done.

================
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()) {
----------------
djasper wrote:
> What happened here?
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.

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

================
Comment at: lib/Format/Format.cpp:1544
@@ -1508,2 +1543,3 @@
     deriveLocalStyle(AnnotatedLines);
-    computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end());
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
+                                          AnnotatedLines.end());
----------------
djasper wrote:
> Move this into the base class?
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.


Repository:
  rL LLVM

http://reviews.llvm.org/D18551





More information about the cfe-commits mailing list