[PATCH] D41102: Setup clang-doc frontend framework

Julie Hockett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 27 14:36:32 PST 2018


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





More information about the cfe-commits mailing list