[PATCH] D26016: BitcodeReader: Require clients to read the block info block at most once.

[Mehdi AMINI via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D26016#581335, @jordan_rose wrote:

> That's how I read it too, but that's reasonable behavior if you have several data blobs of the same kind. Your file can be { MAGIC_NUMBER, { BLOCKINFO, DATA, DATA }, { BLOCKINFO, DATA, DATA }, …} instead of { MAGIC_NUMBER, BLOCKINFO, { DATA, DATA }, { DATA, DATA}, …}. Why is the former better for concatenation than the latter? Because you don't need to know the size of the BLOCKINFO.
>
> It's not a //huge// advantage, but someone could certainly be making use of it, and this will break them. (I don't think we're currently using it but I could check.)


If someone would rely on it, I'd rather have a mechanism to validate that the second BLOCKINFO is identical. This seems too much error prone to just ignore it.

> Again, if the two BLOCKINFO structures are different then you can't reuse the cursor today, so this is effectively no change there. I guess it helps catch reuse mistakes.

Right!



================
Comment at: llvm/lib/Bitcode/Reader/BitstreamReader.cpp:323
+  // We expect the client to read the block info block at most once.
+  assert(!getBitStreamReader()->hasBlockInfoRecords());
 
----------------
Can we have an error instead of an assert?


https://reviews.llvm.org/D26016