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