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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to