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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to