martong marked 8 inline comments as done.
martong added a comment.
Alexei, thanks for the review!
================
Comment at: lib/AST/ASTImporter.cpp:3002
+ // Check if we have found an existing definition. Returns with that
+ // definition if yes, otherwise returns null.
----------------
a_sidorin wrote:
> 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.
Ok, I have created a member function in ASTNodeImporter instead of the lambda.
================
Comment at: unittests/AST/ASTImporterTest.cpp:3862
+ void CheckPreviousDecl(Decl *To0, Decl *To1) {
+ ASSERT_NE(To0, To1);
----------------
a_sidorin wrote:
> I don't like numbers. Maybe `To0` and `To1` are `LastDecl` and
> `ImportedDecl`, correspondingly?
Ok, I have renamed To0 to Prev and To1 to Current as they should form a redecl
chain this way: Prev->Current.
================
Comment at: unittests/AST/ASTImporterTest.cpp:3881
+ if (auto *From0F = dyn_cast<FunctionDecl>(To0)) {
+ auto *To0F = cast<FunctionDecl>(To0);
+ if (From0F->getTemplatedKind() ==
----------------
a_sidorin wrote:
> To0 and From0F actually have the same value, and To0F is unused.
Good catch. I removed the second variable.
================
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.
----------------
a_sidorin wrote:
> 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.
Yes, thanks for catching this. I have moved this upward right at the beginning
of the function.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58668/new/
https://reviews.llvm.org/D58668
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
