[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
The patch looks almost good bu I have some comments inline.



================
Comment at: lib/AST/ASTImporter.cpp:3002
 
+  // Check if we have found an existing definition.  Returns with that
+  // definition if yes, otherwise returns null.
----------------
I like this lambda. To make the code even better, we can move this lambda outside of VisitFunctionDecl because this method is already pretty big.


================
Comment at: unittests/AST/ASTImporterTest.cpp:3862
 
+  void CheckPreviousDecl(Decl *To0, Decl *To1) {
+    ASSERT_NE(To0, To1);
----------------
I don't like numbers. Maybe `To0` and `To1` are `LastDecl` and `ImportedDecl`, correspondingly?


================
Comment at: unittests/AST/ASTImporterTest.cpp:3881
+    if (auto *From0F = dyn_cast<FunctionDecl>(To0)) {
+      auto *To0F = cast<FunctionDecl>(To0);
+      if (From0F->getTemplatedKind() ==
----------------
To0 and From0F actually have the same value, and To0F is unused.


================
Comment at: unittests/AST/ASTImporterTest.cpp:3884
+          FunctionDecl::TK_FunctionTemplateSpecialization) {
+        EXPECT_EQ(To0->getCanonicalDecl(), To1->getCanonicalDecl());
+        // There may be a hidden fwd spec decl before a spec decl.
----------------
If I don't miss something, this assumption does not depend on To0 and To1 kind and can be moved out of the condition, to the function scope.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58668