sepavloff added a comment. I still cannot build project with your changes, now compiler cannot find symbol `hasSubStmt`. When committing the change please make sure that all prerequisites are committed and unit tests run successfully.
I would recommend you to take tests from http://reviews.llvm.org/D14224 for literals. They check imported values as well, not only syntax structure. It is however only a recommendation. There is no tests for `InjectedClassNameType`, `TemplateTypeParmType`, `GCCAsmStmt`, `VAArgExpr` and `CXXBoolLiteralExpr`. Probably these nodes may be moved to next code change? Provided that the aforementioned nodes get tests of are moved to another change, the patch looks good for me. I do not have authority to approve patches for commit, you need to ask someone with proper rights for the approval. Thank you! ================ Comment at: lib/AST/ASTImporter.cpp:1827-1828 @@ +1826,4 @@ + + // FIXME: ASTContext::getInjectedClassNameType is not suitable for AST reading + // See comments in its definition for details + // return Importer.getToContext().getInjectedClassNameType(D, InjType); ---------------- Definition of `ASTContext::getInjectedClassNameType` does not contain comments that would explain why it is unsuitable, so this comment is confusing. Probably you wanted to reference `ASTReader::readTypeRecord`, it contains similar statement. ================ Comment at: lib/AST/ASTImporter.cpp:4379 @@ +4378,3 @@ + + // Resolve possible cyclic import + if (Decl *AlreadyImported = Importer.GetAlreadyImportedOrNull(D)) ---------------- Comments should be proper English sentences. In this case ending period is missed. ================ Comment at: lib/AST/ASTImporter.cpp:4730 @@ +4729,3 @@ + SmallVector<IdentifierInfo *, 4> Names; + for (unsigned i = 0, e = S->getNumOutputs(); i != e; i++) { + IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(i)); ---------------- According to LLVM coding style ([[ http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly ]]) variable names 'should be camel case, and start with an upper case letter', so `i` -> `I`, `e` -> `E`. ================ Comment at: unittests/AST/ASTImporterTest.cpp:117 @@ +116,3 @@ + hasType( + asString("const char [4]"))))))))); + EXPECT_TRUE(testImport("void declToImport() { L\"foo\"; }", ---------------- Disadvantage of using matchers, apart of complexity, is that they check only syntax structure. This test does not check if imported literal has proper value. Repository: rL LLVM http://reviews.llvm.org/D14286 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
