[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#581319, @jordan_rose wrote:

> BLOCKINFO is part of the container format <http://llvm.org/docs/BitCodeFormat.html>, not part of an LLVM module.


OK, I was looking at how we use it today in LLVM.

> I'm objecting to the idea that it's better to write one BLOCKINFO for several "data blobs" (LLVM module, Swift module, whatever) than to just stick the BLOCKINFO in each data blob and be done with it. Right now we have a completely concatenative format, as long as you separate out the fixed-size magic number at the start; this would break that.

Can you clarify how this concatenation works? I'm puzzled because my impression is that today if you try to stick a blockinfo in each blob, it will *ignore* the second one. This how I read the code modified in this patch:

  // If this is the second stream to get to the block info block, skip it.
  // We expect the client to read the block info block at most once.
  assert(!getBitStreamReader()->hasBlockInfoRecords());

calling:

  /// Return true if we've already read and processed the block info block for
  /// this Bitstream. We only process it for the first cursor that walks over
  /// it.
  bool hasBlockInfoRecords() const { return !BlockInfoRecords.empty(); }

Maybe you are you saying that we should support this use case? (I may have mis-read your objection from the beginning as something that this patch would break).


https://reviews.llvm.org/D26016