Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 | Can we get at the SubExprs and verify them as well? | |
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 | Sure! What's the policy on this? I can see test, just like ImportChooseExpr that simply checks for the existence of the node. | |
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 | The best if we can check as much constituent of the AST node as possible in the test. Historically, testImport is among the first testing facilities. Later, we developed ASTImporterTestBase::Import that provides you the imported decl and from that point you can formulate any kind of expectations on that decl. I'd like to advise you to not use testImport unless the node is trivial. Here is a good and simple example for Import: TEST_P(ASTImporterOptionSpecificTestBase, ImportEnumMemberSpecialization) {
Decl *FromTU = getTuDecl(
R"(
template <class T> struct A {
enum tagname { enumerator };
};
template struct A<int>;
)",
Lang_CXX03);
auto *FromD = FirstDeclMatcher<EnumDecl>().match(
FromTU, enumDecl(hasName("tagname"),
hasParent(classTemplateSpecializationDecl())));
ASSERT_TRUE(FromD);
ASSERT_TRUE(FromD->getMemberSpecializationInfo());
auto *ToD = Import(FromD, Lang_CXX03);
EXPECT_TRUE(ToD);
EXPECT_TRUE(ToD->getMemberSpecializationInfo());
EXPECT_EQ(FromD->getTemplateSpecializationKind(),
ToD->getTemplateSpecializationKind());
} | |
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 | Uhh, ohh, I see that Import is implemented for Decls only, perhaps we should have something similar to Stmts as well ... | |
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 |
I don't think Stmt's have a reference to their containing function so I don't think that can be implemented? | |
Use a more elaborate matcher to verify subexpressions.
Also fix a bug: reserve() -> resize() on the destination vector.
I don't think there is a great (or any) benefit for running tests on patches like this so don't feel obligated to run the tests. LLDB doesn't construct Stmts from debug info, so the only way we could even encounter most of these specialized Stmts in LLDB is if 1. we have a dedicated test for it (which is easy to grep for but as there are no similar tests I doubt you'll find results for that) 2. They show up in a system header which LLDB parses during runtime. All the tests that do this on Linux are in the import-std-module directory so you could just run that if you really want to run the tests before. But as those are system headers you would anyway also rely on the bots to discover different versions of system headers.
I would just grep for the class/source representation in the LLDB directory and if they aren't explicitly referenced in a test/source then I would just leave it up to the bots to run the tests.
Looks good! Thanks!
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 |
That's right, but I think we could implement a convenience function in the ASTImporterTestBase that can return the imported Stmt*, i.e. Stmt* ASTImporterTestBase::Import(Stmt*). And once we have that then we can check all the needed properties and we don't have to formulate complex matchers. | |
| clang/unittests/AST/ASTImporterTest.cpp | ||
|---|---|---|
| 304 | I probably should have phrased 'can't be implemented' as 'can't be implemented nicely'. My point was more that the Import(Stmt *) implementation would either
| |
Can we get at the SubExprs and verify them as well?