ilya-biryukov added a comment.
Mostly nits, but could you elaborate why can't we print the type-loc when
printing the template argument (see the corresponding comment)?
================
Comment at: clang-tools-extra/clangd/AST.cpp:66
+ return Var->getTemplateArgsInfo().arguments();
+ return llvm::None;
+}
----------------
NIT: add a note that we return null for `ClassTemplateSpecializationDecl `
because we can't construct an `ArrayRef` for it
================
Comment at: clang-tools-extra/clangd/AST.cpp:84
+ if (auto STL = TL.getAs<TemplateSpecializationTypeLoc>()) {
+ std::vector<TemplateArgumentLoc> ArgLocs;
+ for (unsigned I = 0; I < STL.getNumArgs(); I++)
----------------
NIT: use `SmallVector<8>` or some other small-enough number to avoid most
allocs.
================
Comment at: clang-tools-extra/unittests/clangd/IndexTests.cpp:18
#include "clang/Index/IndexSymbol.h"
+#include "gmock/gmock-generated-matchers.h"
#include "gmock/gmock.h"
----------------
NIT: maybe remove it? clangd keeps adding those, but I don't think we actually
want it: `gmock.h` should be enough
================
Comment at: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp:417
+ ForCodeCompletion(true)),
+ AllOf(QName("Tmpl<int, bool, Bar, 3>"),
+ DeclRange(Header.range("specdecl")), ForCodeCompletion(false)),
----------------
Does it mean typing `bool` could potentially match `vector<bool>` in
`workspaceSymbols` now?
If that's the case we might start producing some noise.
================
Comment at: clang/lib/AST/TypePrinter.cpp:1644
+ llvm::raw_ostream &OS) {
+ A.getTypeSourceInfo()->getType().print(OS, PP);
+}
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Maybe print the result of `getTypeLoc()` here, if it's available?
> > Would produce results closer to the ones written in the code.
> unfortunately it is not available.
you mean the function to print a type loc or the type loc itself?
================
Comment at: clang/lib/AST/TypePrinter.cpp:1646
+ break;
+ case TemplateArgument::ArgKind::Type:
+ A.getTypeSourceInfo()->getType().print(OS, PP);
----------------
Maybe simplify the switch to:
```
if (A.getKind() == TemplateArgument::ArgKind::Type) {
A.getTypeSourceInfo()->getType().print(OS, PP);
return;
}
A.getArgument().print(PP, OS);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59354/new/
https://reviews.llvm.org/D59354
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
