[PATCH] D43809: Add possibility to specify output stream for CompilerInstance

[Raphael Isemann via Phabricator via cfe-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

See the inline comments. Please run clang-format over this patch after addressing them (there are a few indentation issues).



================
Comment at: include/clang/Frontend/CompilerInstance.h:779
 
+  void SetOutputStream(std::unique_ptr<llvm::raw_pwrite_stream> OutStream) {
+    OutputStream = std::move(OutStream);
----------------
"Set" -> "set", LLVM code style requires lower case first letters in function names (even though it's sadly may be inconsistent with other code that's still using the other style...)


================
Comment at: include/clang/Frontend/CompilerInstance.h:783
+
+  std::unique_ptr<llvm::raw_pwrite_stream> GetOutputStream() {
+    return std::move(OutputStream);
----------------
LLVM convention is to call this `takeX` and not `getX` when it moves the member `OutputStream` to the caller. `get` usually doesn't reset the member variable.


================
Comment at: lib/Frontend/CompilerInstance.cpp:65
+      ThePCHContainerOperations(std::move(PCHContainerOps)),
+      OutputStream(nullptr) {
   // Don't allow this to invalidate buffers in use by others.
----------------
Don't think we need that.


Repository:
  rC Clang

https://reviews.llvm.org/D43809