a.sidorin marked 7 inline comments as done.
a.sidorin added a comment.
Thank you for your comments. I leaved some replies and will update revision.
Something about lacking tests.
1. We cannot check if expression import is correct until we implement
FunctionDecl body import. I was going to upstream Decl-related parts later but
maybe it is better to include this small case in the first patch. What do you
think?
2. What is the best way to test import of statements? Currently I have no idea
how to do this (except of ast-dump). Any suggestions are welcome.
================
Comment at: lib/AST/ASTImporter.cpp:35
@@ +34,3 @@
+ void ImportMultipleItems(IIter Ibegin, IIter Iend, OIter Obegin) {
+ ASTImporter &_Importer = Importer;
+ std::transform(Ibegin, Iend, Obegin,
----------------
sepavloff wrote:
> A name started with underscore is a reserved identifier (see C++ standard,
> [global.names]), so it is better to use something more neutral, like
> //TheImporter// or //Imp// or something else.
Ouch. I was just trying to unify this with the code already existing in
ASTImporter. I'll rename this.
// TODO: refactor std::transform usage in ASTImporter.
================
Comment at: lib/AST/ASTImporter.cpp:48
@@ +47,3 @@
+ template<typename IIter, typename OIter>
+ bool checkPossibleNull(IIter Ibegin, IIter Iend, OIter Obegin) {
+ for (; Ibegin != Iend; Ibegin++, Obegin++)
----------------
sepavloff wrote:
> This function is used in one place only, maybe inline its body in that place?
I'll use it in later patches so I'd prefer to keep it.
================
Comment at: lib/AST/ASTImporter.cpp:4655
@@ +4654,3 @@
+ for (unsigned i = 0, e = S->getNumClobbers(); i != e; i++) {
+ StringLiteral *Clobber = cast_or_null<StringLiteral>(
+ Importer.Import(S->getClobberStringLiteral(i)));
----------------
sepavloff wrote:
> Again, clobber cannot be null, maybe `cast` instead of `cast_or_null`?
This guard is here because the return result of import may be null. This
cast_or_null (and some others) handles such cases.
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
