[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