[PATCH] D41102: Setup clang-doc frontend framework

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added inline comments.


================
Comment at: clang-doc/BitcodeWriter.cpp:407
+
+void ClangDocBinaryWriter::writeBitstream(const EnumInfo &I,
+                                          BitstreamWriter &Stream,
----------------
juliehockett wrote:
> jakehehrlich wrote:
> > lebedev.ri wrote:
> > > juliehockett wrote:
> > > > lebedev.ri wrote:
> > > > > Hmm, common pattern again
> > > > > ```
> > > > > void ClangDocBinaryWriter::writeBitstream(const <TYPENAME> &I,
> > > > >                                           BitstreamWriter &Stream,
> > > > >                                           bool writeBlockInfo) {
> > > > >   if (writeBlockInfo) emitBlockInfoBlock(Stream);
> > > > >   StreamSubBlock Block(Stream, BI_<ENUM_NAMETYPE>_BLOCK_ID);
> > > > >   ...
> > > > > }
> > > > > ```
> > > > > Could be solved if a mapping from `TYPENAME` -> `BI_<ENUM_NAMETYPE>_BLOCK_ID` can be added.
> > > > > If LLVM would be using C++14, that'd be easy, but with C++11, it would require whole new class (although with just a single static variable).
> > > > Do you want me to try to write that class, or leave it as it is?
> > > It would be something like: (total guesswork, literally just wrote it here, like rest of the snippets)
> > > ```
> > > template <typename TypeInfo>
> > > struct MapFromTypeToEnumerator {
> > >   static const BlockId id;
> > > };
> > > 
> > > template <>
> > > struct MapFromTypeToEnumerator<NamespaceInfo> {
> > >   static const BlockId id = BI_NAMESPACE_BLOCK_ID;
> > > };
> > > void ClangDocBitcodeWriter::writeBitstream(const NamespaceInfo &I) {
> > >   EMITINFO(NAMESPACE)
> > > }
> > > ...
> > > 
> > > template <typename TypeInfo>
> > > void ClangDocBitcodeWriter::writeBitstream(const TypeInfo &I, bool writeBlockInfo) {
> > >   if (writeBlockInfo) emitBlockInfoBlock();
> > >   StreamSubBlockGuard Block(Stream, MapFromTypeToEnumerator<TypeInfo>::id);
> > >   writeBitstream(I);
> > > }
> > > ```
> > > Uhm, now that i have wrote it, it does not look as ugly as i though it would look...
> > > So maybe try integrating that, i *think* it is a bit cleaner?
> > If we know the set of types then it should just be a static member of every *Info type. Then the mapping is just TYPENAME::id
> @jakehehrlich The issue with that is that it would mix writer implementation details into the representation, which at this point has no knowledge of the writer. We can break that, but is that the best option?
Probably not, I didn't think about that.


https://reviews.llvm.org/D41102