[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+  if (const auto *TD = dyn_cast<T>(D))
+    return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+  return false;
----------------
ilya-biryukov wrote:
> jvikstrom wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > We also want to skip `TSK_ExplicitInstantiationDeclaration` and  `TSK_ExplicitInstantiationDefinition` here.
> > > > > This covers cases like (not sure which one of the two enum values we get, though):
> > > > > ```
> > > > > template <class T>
> > > > > int foo(T) { ... }
> > > > > 
> > > > > template int foo(int); // we'd rather not traverse these, highlightings will run into the same problems.
> > > > > ```
> > > > > 
> > > > > Semantics I'm describing are roughly similar to `isImplicitInstatiation(D) == !isExplicitInstantion(D)`, where `isExplicitInstantiation` is taken from `CodeComplete.cpp`. (If we ignore `TSK_Undeclared`, which, I believe, should never be encountered in decls passed to HandleTopLevelDecl).
> > > > > 
> > > > > Please extract the helper from code complete and this one into a separate file (e.g. `clangd/AST.h`) and possibly implement one through the other
> > > > > Semantics I'm describing are roughly similar to isImplicitInstatiation(D) == !isExplicitInstantion(D), 
> > > > 
> > > > I think there is a typo here, I believe you mean `isImplicitInstantiation(D) == !isExplicitSpecialization(D) ` (in CodeComplete.cpp, it checks whether a Decl is an explicit **specialization**).
> > > Yes, there's a typo thanks!
> > Still want me to move this and the helper from CodeComplete to `AST.h` as it isn't used anywhere else? (esp. when we can't implement them through of each other)
> Yes, it's better to share this code between `codeComplete` and `ClangdUnit`. It might pop up in more places and it's not trivial.
Sorry, I mean **share functions from the same place** between code complete and clangd unit.
They might not be implementable through each other, but they are very similar and having them in the same place means that we will have easier time keeping them both correct in case we need to change any of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65510