[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-doc/ClangDocBinary.h:82
+
+static std::map<BlockId, std::string> BlockIdNameMap = {
+  {NAMESPACE_BLOCK_ID, "NamespaceBlock"},
----------------
lebedev.ri wrote:
> Nice!
> Some thoughts:
> 1. I agree it makes sense to keep it close to the enum definition, in header...
> 2. This will result in global constructor. Generally they are frowned upon in LLVM. But since this is a standalone binary, it may be ok?
> 3. Have you tried using `StringRef` here, instead of `std::string`?
> 4. `std::map` is in general a bad idea.
>       Since the `enum`'s enumerators are all small and consecutive, maybe try `llvm::IndexedMap`?
Also, this should be `static const`, since the underlying enum won't change on the fly.

`#llvm` suggests to use TableGen here, i'm not sure how that would work.

As i have now noticed, there isn't a init-list constructor, so I think **something** like this might work:
```
static const llvm::IndexedMap<BlockId> BlockIdNameMap = []() {
  llvm::IndexedMap<BlockId> map;
  map.reserve(BI_LAST);

  // There is no init-list constructor for the IndexedMap, so have to improvise
  static const std::initializer_list<std::pair<BlockId, const char* const>> inits = {
    {NAMESPACE_BLOCK_ID, "NamespaceBlock"},
    ...
  };
  for(const auto& init : inits)
    map[init.first] = init.second;
}();
```

Also, even though `llvm::IndexedMap<>` is using `llvm::SmallVector<>` internally, it does not expose the initial size as template parameter, unfortunately, but hardcodes it to `0`. I think it would be great to add one more template parameter to `llvm::IndexedMap<>`, which would default to `0`, but would allow us here to avoid all memory allocation altogether.

What do you think? If you do agree that using `IndexedMap` seems like the right choice, but **don't** want to write the patch for template parameter, i might look into it..


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41102