juliehockett added inline comments.
================
Comment at: clang-doc/BitcodeWriter.cpp:219
+
+void ClangDocBitcodeWriter::emitIntRecord(int Value, RecordId ID) {
+ if (!Value) return;
----------------
lebedev.ri wrote:
> Now, all these three `emit*Record` functions now have the 'same signature':
> ```
> template <typename T>
> void ClangDocBitcodeWriter::emitRecord(const T& Record, RecordId ID);
>
> template <>
> void ClangDocBitcodeWriter::emitRecord(StringRef Str, RecordId ID) {
> ...
> ```
>
> **Assuming there are no implicit conversions going on**, i'd make that change.
> It, again, may open the road for further generalizations.
I overloaded the functions -- cleaner, and deals with any implicit conversions
nicely.
================
Comment at: clang-doc/BitcodeWriter.h:178
+ void emitTypeBlock(const std::unique_ptr<TypeInfo> &N);
+ void emitMemberTypeBlock(const std::unique_ptr<MemberTypeInfo> &N);
+ void emitFieldTypeBlock(const std::unique_ptr<FieldTypeInfo> &N);
----------------
lebedev.ri wrote:
> Let's continue cracking down on duplication.
> I think these four functions need the same template treatment as
> `writeBitstreamForInfo()`
>
> (please feel free to use better names)
> ```
> template<typename T>
> void emitBlock(const std::unique_ptr<T> &B);
>
> template<typename T>
> void emitTypedBlock(const std::unique_ptr<T> &B) {
> StreamSubBlockGuard Block(Stream, MapFromInfoToBlockId<T>::ID);
> emitBlock(B);
> }
>
> template<>
> void ClangDocBitcodeWriter::emitBlock(const std::unique_ptr<TypeInfo> &T) {
> emitStringRecord(T->TypeUSR, FIELD_TYPE_TYPE);
> for (const auto &CI : T->Description) emitCommentBlock(CI);
> }
> ```
>
> I agree that it seems strange, and seem to actually increase the code size so
> far,
> but i believe by exposing similar functionality under one function,
> later, it will open the road for more opportunities of further consolidation.
Since it actually ended up duplicating the `writeBitstreamForInfo()` code, I
rolled all of this into one `emitBlock()` entry point.
https://reviews.llvm.org/D41102
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
