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