[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM outside of the question I had.



================
Comment at: lib/AST/ASTImporter.cpp:4967
+template <typename T> static auto getTemplateDefinition(T *D) -> T * {
+  auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
   if (!ToTemplatedDef)
----------------
Can we guarantee that `D->getTemplatedDecl()` will always return a valid pointer? 


================
Comment at: lib/AST/ASTImporter.cpp:4967
+template <typename T> static auto getTemplateDefinition(T *D) -> T * {
+  auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
   if (!ToTemplatedDef)
----------------
shafik wrote:
> Can we guarantee that `D->getTemplatedDecl()` will always return a valid pointer? 
What other types besides `CXXRecordDecl` do we expect here? 


================
Comment at: lib/AST/ASTImporter.cpp:5544
   // type, and in the same context as the function we're importing.
+  // FIXME Split this into a separate function.
   if (!LexicalDC->isFunctionOrMethod()) {
----------------
Would it make sense to do the split into a separate function in the PR?


================
Comment at: lib/AST/ASTImporter.cpp:5595
+      auto *PrevTemplated =
+          FoundByLookup->getTemplatedDecl()->getMostRecentDecl();
+      if (TemplatedFD != PrevTemplated)
----------------
Can we guarantee that `FoundByLookup->getTemplatedDecl()` will always return a valid pointer? 


Repository:
  rC Clang

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

https://reviews.llvm.org/D58494