[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In Swift's IRGen, we have an option controlling whether to emit meaningful value names.  That option can be set directly from the command line, but it defaults to whether we're emitting IR assembly (i.e. .ll, not .bc), which means that the clients most interested in getting meaningful value names — humans, presumably — always get good names, but nobody else pays for them.  I have many, many times wished that we'd taken that same approach in Clang instead of basing it on build configuration.

If type names are a significant burden on compile times, should we just start taking that same approach?  Just don't use real type names in .bc output unless we're asked to.  Maybe we can eventually retroactively use the same option for value names.

I agree with Reid that it's really weird for there to be a raw_ostream that automatically rewrites the string contents to be a hash when some limit is reached; that does not feel like generalizable technology.



================
Comment at: include/clang/AST/PrettyPrinter.h:231
+  /// make things like breaking digraphs and '>>' in template parameters.
+  bool NotForCompilation : 1;
 };
----------------
This saves, what, a few spaces from a few thousand IR type names?  I'm skeptical that this even offsets the code-size impact of adding this option.


================
Comment at: include/clang/AST/Type.h:4623
+                               const TemplateArgumentListInfo &Args,
+                               const PrintingPolicy &Policy);
+
----------------
I like this refactor, but since it's the majority of your patch, please split it out (it can use post-commit review) and make this patch just about your actual change.


================
Comment at: lib/AST/TypePrinter.cpp:1543
+  for (const auto &Arg : Args) {
+    std::unique_ptr<SmallString<128>> Buffer;
+    std::unique_ptr<llvm::raw_svector_ostream> BufOS;
----------------
rnk wrote:
> Just use `SmallString<0>` for Buffer. No wasted stack space, no extra unique_ptr. It will allocate memory if it needs it.
Well, it might as well have some stack storage, but otherwise I agree.


https://reviews.llvm.org/D40508