[PATCH] D30266: Move Stream code from MSF -> Support
Eric Christopher via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 24 11:11:28 PST 2017
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
Couple of inline comments while I was reading through. Hope we can use this to clean up some of the reading/writing interfaces in future patches :)
-eric
================
Comment at: llvm/include/llvm/Support/BinaryStreamReader.h:30-31
+/// null-terminated strings, integers in various flavors of endianness, etc.
+/// Can be subclassed to provide reading of custom datatypes, although no
+/// are overridable.
+class BinaryStreamReader {
----------------
Grammar.
================
Comment at: llvm/lib/Support/BinaryStreamWriter.cpp:28
+
+Error BinaryStreamWriter::writeZeroString(StringRef Str) {
+ if (auto EC = writeFixedString(Str))
----------------
This seems like you want to say "writeTerminatedString"?
I'd probably go with "writeCString" or "writeString" and use "writeUnterminatedString"? Or something?
================
Comment at: llvm/lib/Support/BinaryStreamWriter.cpp:42
+
+ Offset += Str.size();
+ return Error::success();
----------------
Don't we already update the offset in writeBytes?
https://reviews.llvm.org/D30266
More information about the llvm-commits
mailing list