[PATCH] D59354: [clangd] Print arguments in template specializations

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 19 03:16:58 PDT 2019


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few NITs.
Thanks for fixing this!



================
Comment at: clang-tools-extra/clangd/AST.cpp:86
+    auto TL = Cls->getTypeAsWritten()->getTypeLoc();
+    if (auto STL = TL.getAs<TemplateSpecializationTypeLoc>()) {
+      llvm::SmallVector<TemplateArgumentLoc, 8> ArgLocs;
----------------
NIT: maybe inline `TL`?
```
if (auto STL = Cls->getTypeAsWritten()->getTypeLoc().getAs<TemplateSpecializationTypeLoc>()) {
  /// ...
}
```


================
Comment at: clang-tools-extra/clangd/AST.cpp:89
+      ArgLocs.reserve(STL.getNumArgs());
+      for (unsigned I = 0; I < STL.getNumArgs(); I++)
+        ArgLocs.push_back(STL.getArgLoc(I));
----------------
Tiny NIT: [[ https://llvm.org/docs/CodingStandards.html#prefer-preincrement | prefer preincrement ]]


================
Comment at: clang/lib/AST/TypePrinter.cpp:1635
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+                          llvm::raw_ostream &OS) {
----------------
NIT: consider also adding a test in the clang code.
Some developers don't build `tools-extra`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59354/new/

https://reviews.llvm.org/D59354





More information about the cfe-commits mailing list