martong added inline comments.
================
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()))));
----------------
a.sidorin wrote:
> Tests with `has(fieldDecl())` duplicate tests with `hasFieldOrder()`. Is it
> intentional?
Ok, I removed the redundant `has(fieldDecl())` checks.
================
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"}))));
----------------
a.sidorin wrote:
> Should this be `match(From, ...)` instead of `To`?
That's right, good catch.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1177
+ ASTImporterTestBase,
+
DISABLED_CXXRecordDeclFieldsShouldBeInCorrectOrderEvenWhenWeImportFirstTheLastDecl)
{
+ Decl *From, *To;
----------------
a.sidorin wrote:
> This identifier is very long. How about renaming it to
> DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder? It is 60 char
> long instead of 82.
Ok, I like the shorter name, changed it.
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
