rnk added a comment.
It's not clear to me that this abbreviation functionality should live in
Support. Clang probably wants enough control over this (assuming we're doing
it) that the logic should live in clang.
I also think we might want to try solving this a completely different way: just
don't bother emitting more than two template arguments for IR type names. We
don't really need to worry about type name uniqueness or matching them across
TUs.
================
Comment at: lib/AST/TypePrinter.cpp:1532-1534
+namespace {
+template<typename TA>
+void printTo(raw_ostream &OS, ArrayRef<TA> Args, const PrintingPolicy &Policy,
----------------
`static` is nicer than a short anonymous namespace.
================
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;
----------------
Just use `SmallString<0>` for Buffer. No wasted stack space, no extra
unique_ptr. It will allocate memory if it needs it.
================
Comment at: lib/AST/TypePrinter.cpp:1588-1589
-// Sadly, repeat all that with TemplateArgLoc.
-void TemplateSpecializationType::
-PrintTemplateArgumentList(raw_ostream &OS,
- ArrayRef<TemplateArgumentLoc> Args,
- const PrintingPolicy &Policy) {
- OS << '<';
- const char *Comma = Policy.MSVCFormatting ? "," : ", ";
-
- bool needSpace = false;
- bool FirstArg = true;
- for (const TemplateArgumentLoc &Arg : Args) {
- if (!FirstArg)
- OS << Comma;
-
- // Print the argument into a string.
- SmallString<128> Buf;
- llvm::raw_svector_ostream ArgOS(Buf);
- if (Arg.getArgument().getKind() == TemplateArgument::Pack) {
- PrintTemplateArgumentList(ArgOS,
- Arg.getArgument().getPackAsArray(),
- Policy, true);
- } else {
- Arg.getArgument().print(Policy, ArgOS);
- }
- StringRef ArgString = ArgOS.str();
-
- // If this is the first argument and its string representation
- // begins with the global scope specifier ('::foo'), add a space
- // to avoid printing the diagraph '<:'.
- if (FirstArg && !ArgString.empty() && ArgString[0] == ':')
- OS << ' ';
-
- OS << ArgString;
-
- needSpace = (!ArgString.empty() && ArgString.back() == '>');
- FirstArg = false;
- }
+namespace clang {
+void printTemplateArgumentList(raw_ostream &OS,
+ const TemplateArgumentListInfo &Args,
----------------
It's usually nicer to define free functions in namespaces as `void
clang::printTemplateArgumentList(...`. This ensures that nobody can mess up the
namespace scope or forget to include the header that declares the function. It
also sometimes turns link errors into compile errors.
https://reviews.llvm.org/D40508
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
