[PATCH] D86905: Flush bitcode incrementally for LTO output

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 31 21:08:59 PDT 2020


MaskRay added a comment.

I can understand the read-write stream requirement, but the changes to lib/Support may require an additional set of reviewers. You will need some unittests (see `llvm/unittests/Support/raw_ostream_test.cpp` for example)



================
Comment at: lld/ELF/LTO.cpp:69
+  if (!ec) {
+    return fs;
+  }
----------------
Avoid braces around simple statements

http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: lld/ELF/LTO.cpp:71
+  }
+  return openFile(file);
+}
----------------
`return {};`

openFile would likely fail for the same reason.


================
Comment at: llvm/include/llvm/Support/raw_ostream.h:318
+
+  virtual Kinds get_kind() const;
+
----------------
https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html

We usually use static `classof`


================
Comment at: llvm/lib/Support/raw_ostream.cpp:32
 #include <iterator>
+#include <stdexcept>
 #include <sys/stat.h>
----------------
Can this be avoided?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86905/new/

https://reviews.llvm.org/D86905



More information about the llvm-commits mailing list