[PATCH] D41102: Setup clang-doc frontend framework

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 9 02:58:51 PST 2018


lebedev.ri added a comment.

Might have been better to not start landing until the all differentials are understood/accepted, but i understand that it is not really up to me to decide.
Let's hope nothing in the next differentials will require changes to this initial code :)



================
Comment at: clang-doc/BitcodeWriter.h:241
+/// \param I The info to emit to bitcode.
+template <typename T> void ClangDocBitcodeWriter::emitBlock(const T &I) {
+  StreamSubBlockGuard Block(Stream, MapFromInfoToBlockId<T>::ID);
----------------
sammccall wrote:
> OK, I don't get this at all.
> 
> We have to declare emitBlockContent(NamespaceInfo) *and* the specialization of MapFromInfoToBlockId<NamespaceInfo>, and deal with the public interface emitBlock being a template function where you can't tell what's legal to pass, instead of writing:
> 
> ```void emitBlock(const NamespaceInfo &I) {
>   SubStreamBlockGuard Block(Stream, BI_NAMESPACE_BLOCK_ID); // <-- this one line
>   ...
> }```
> 
> This really seems like templates for the sake of templates :(
If you want to add a new block, in one case you just need to add one
```
template <> struct MapFromInfoToBlockId<???Info> {
  static const BlockId ID = BI_???_BLOCK_ID;
};
```
In the other case you need to add whole
```
void ClangDocBitcodeWriter::emitBlock(const ???Info &I) {
  StreamSubBlockGuard Block(Stream, BI_???_BLOCK_ID);
  emitBlockContent(I);
}
```
(and it was even longer initially)
It seems just templating one static variable is shorter than duplicating `emitBlock()` each time, no?

Do compare the current diff with the original diff state.
I *think* these templates helped move much of the duplication to simplify the code overall.


Repository:
  rL LLVM

https://reviews.llvm.org/D41102





More information about the cfe-commits mailing list