a.sidorin added a comment.
Hi Gabor! This looks much better.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1000
+ R"(
+ template<typename _T>
+ struct X {};
----------------
In C++, names started with underscore are reserved for implementation so it's
undesirable to use them.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1003
+
+ void declToImport(int y,X<int> &x){}
+
----------------
I still can see a number of style violations in inline code. Some examples:
lack of space between arguments and parameters, no space before '{' in `g(){`,
etc. Could you please fix them?
================
Comment at: unittests/AST/ASTImporterTest.cpp:1169
+ MatchVerifier<Decl> Verifier;
+ ASSERT_TRUE(Verifier.match(From, cxxRecordDecl(has(fieldDecl()))));
+ ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl()))));
----------------
Tests with `has(fieldDecl())` duplicate tests with `hasFieldOrder()`. Is it
intentional?
================
Comment at: unittests/AST/ASTImporterTest.cpp:1171
+ ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl()))));
+ ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"}))));
+ EXPECT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"}))));
----------------
Should this be `match(From, ...)` instead of `To`?
================
Comment at: unittests/AST/ASTImporterTest.cpp:1177
+ ASTImporterTestBase,
+
DISABLED_CXXRecordDeclFieldsShouldBeInCorrectOrderEvenWhenWeImportFirstTheLastDecl)
{
+ Decl *From, *To;
----------------
This identifier is very long. How about renaming it to
DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder? It is 60 char
long instead of 82.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1194
+ ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl()))));
+ ASSERT_TRUE(
+ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}))));
----------------
Same as upper.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1211
+ MatchVerifier<Decl> Verifier;
+ // matches the implicit decl
+ auto Matcher = classTemplateDecl(has(cxxRecordDecl(has(cxxRecordDecl()))));
----------------
Please make it a complete sentence.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1258
+ Lang_CXX);
+ Decl* From = LastDeclMatcher<Decl>{}.match(FromTU, functionDecl());
+ const Decl* To = Import(From, Lang_CXX);
----------------
Please stick `*` to the name (below too).
Repository:
rC Clang
https://reviews.llvm.org/D43967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
