[PATCH] D25356: Define PDBFileBuilder::addStream to add stream.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 7 17:11:54 PDT 2016
zturner added inline comments.
================
Comment at: include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h:99-100
msf::MutableByteStream FileInfoBuffer;
+ std::vector<support::ulittle16_t> DbgStreams = {(size_t)DbgHeaderType::Max,
+ (support::ulittle16_t)0};
+ std::vector<std::pair<ArrayRef<uint8_t>, uint32_t>> Streams;
----------------
zturner wrote:
> I wonder if this could just be a `llvm::SmallVector<ArrayRef<uint8_t>, DbgHeaderType::Max>`. This way you don't need one array which indexes into another array, but rather you just have one array altogether that contains all the data.
I believe these should be initialized to `kInvalidStreamIndex`.
================
Comment at: include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h:99-101
+ std::vector<support::ulittle16_t> DbgStreams = {(size_t)DbgHeaderType::Max,
+ (support::ulittle16_t)0};
+ std::vector<std::pair<ArrayRef<uint8_t>, uint32_t>> Streams;
----------------
I wonder if this could just be a `llvm::SmallVector<ArrayRef<uint8_t>, DbgHeaderType::Max>`. This way you don't need one array which indexes into another array, but rather you just have one array altogether that contains all the data.
================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:246-249
uint32_t Length = calculateSerializedLength();
if (auto EC = Msf.setStreamSize(StreamDBI, Length))
return EC;
return Error::success();
----------------
Here you will need to loop through the various optional debug streams and call `setStreamSize()` on each one.
================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:295-296
+ uint32_t Idx = Pair.second;
+ auto Stream =
+ WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
+ StreamWriter Writer(*Stream);
----------------
Here you should check if `Idx == kInvalidStreamIndex`, and `continue` if so.
================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:297
+ WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
+ StreamWriter Writer(*Stream);
+ if (auto EC = Writer.writeArray(Data))
----------------
Can you use a different variable name here? This `Writer` shadows the previous `Writer`, which could be confusing. Maybe calling it `DbgStreamWriter`.
https://reviews.llvm.org/D25356
More information about the llvm-commits
mailing list