a_sidorin added a comment. Hi Balazs,
The patch looks mostly fine. ================ Comment at: unittests/AST/ASTImporterTest.cpp:2256 + FirstDeclMatcher<CXXRecordDecl>().match(FromTU, cxxRecordDecl()); + auto lookup_res = Class->noload_lookup(FromName); + ASSERT_EQ(lookup_res.size(), 0u); ---------------- LLVM naming conventions require it to be `LookupRes`. ================ Comment at: unittests/AST/ASTImporterTest.cpp:2258 + ASSERT_EQ(lookup_res.size(), 0u); + lookup_res = cast<TranslationUnitDecl>(FromTU)->noload_lookup(FromName); + ASSERT_EQ(lookup_res.size(), 1u); ---------------- getTuDecl() already return `TranslationUnitDecl *`. We should just declare `FromTU` as `auto *`, so no cast is needed here. ================ Comment at: unittests/AST/ASTImporterTest.cpp:2279 + EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_TRUE(!To0->isInIdentifierNamespace(Decl::IDNS_Ordinary)); +} ---------------- EXPECT_FALSE can be used instead of negations. ================ Comment at: unittests/AST/ASTImporterTest.cpp:2294 + Lang_CXX, "input0.cc"); + auto From0 = FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern); + auto From1 = LastDeclMatcher<FunctionDecl>().match(FromTU, Pattern); ---------------- Names like FromFriendFunc/FromNormalFunc will make the test more readable. Repository: rC Clang https://reviews.llvm.org/D49798 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
