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



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

Reply via email to