juliehockett added a comment.

In https://reviews.llvm.org/D41102#1020808, @lebedev.ri wrote:

> Ok, great.
>  And it will also complain if you try to output a block within block?


Um...no. Since you can have subblocks within blocks.



================
Comment at: clang-doc/BitcodeWriter.cpp:191
+  Record.clear();
+  for (const char C : BlockIdNameMap[ID]) Record.push_back(C);
+  Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME, Record);
----------------
lebedev.ri wrote:
> juliehockett wrote:
> > lebedev.ri wrote:
> > > Why do we have this indirection?
> > > Is there a need to first to (unefficiently?) copy to `Record`, and then 
> > > emit from there?
> > > Wouldn't this work just as well?
> > > ```
> > > Record.clear();
> > > Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME, 
> > > BlockIdNameMap[ID]);
> > > ```
> > No, since `BlockIdNameMap[ID]` returns a `StringRef`, which can be 
> > manipulated into an `std::string` or a `const char*`, but the `Stream` 
> > wants an `unsigned char`. So, the copying is to satisfy that. Unless 
> > there's a better way to convert a `StringRef` into an array of `unsigned 
> > char`?
> Aha, i see, did not think of that.
> But there is a `bytes()` function in `StringRef`, which returns 
> `iterator_range<const unsigned char *>`. 
> Would it help? 
> http://llvm.org/doxygen/classllvm_1_1StringRef.html#a5e8f22c3553e341404b445430a3b075b
Replaced it with an ArrayRef to the `bytes_begin()` and `bytes_end()`, but that 
only works for the block id, not the record id, since `emitRecordId()` also has 
to emit the ID number in addition to the name in the same record.


================
Comment at: clang-doc/BitcodeWriter.cpp:265
+  if (!prepRecordData(ID, !OmitFilenames)) return;
+  // FIXME: Assert that the line number is of the appropriate size.
+  Record.push_back(Loc.LineNumber);
----------------
lebedev.ri wrote:
> I think it is as simple as
> ```
> assert(Loc.LineNumber < (1U << BitCodeConstants::LineNumberSize));
> ```
> ?
`LineNumber` is  a signed int, so the compiler complains that we're comparing 
signed and unsigned ints.


https://reviews.llvm.org/D41102



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to