nridge added a comment.

Thanks, this is a nice extension of D119537 <https://reviews.llvm.org/D119537>.

I don't have a good sense for how common the "multiple instantiations, same 
type" scenario is; perhaps @Trass3r has an opinion on that.



================
Comment at: clang-tools-extra/clangd/AST.cpp:572
+#define TEMPLATE_TYPE(SomeTemplateDecl)                                        
\
+  if (auto *STD = llvm::dyn_cast<SomeTemplateDecl>(TD)) {                      
\
+    for (auto *Spec : STD->specializations()) {                                
\
----------------
Is it possible to extract this into a helper function template?

I'm not a huge fan of using macros like this, as the macro definition is harder 
to read (e.g. its tokens don't get semantic highlighting) and edit (e.g. 
completion).


================
Comment at: clang-tools-extra/clangd/AST.h:136
+// FIXME: handle more type patterns.
+llvm::Optional<TemplateTypeParmTypeLoc> getContainedAutoParamType(TypeLoc TL);
+
----------------
Why `Optional` if `TypeLoc` can represent a null type loc?


================
Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:303
+    auto AST = TU.build();
+    AST.getASTContext().getTranslationUnitDecl()->dump();
+    PrintingPolicy PP = AST.getASTContext().getPrintingPolicy();
----------------
leftover or deliberate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120258

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

Reply via email to