[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