a.sidorin added a comment.
I' d like to have some tests for this. CXXCtorInitializers can be tested with
D14224 <http://reviews.llvm.org/D14224>-like stuff or with ASTMatchers
(ASTImporterTest.cpp).
Some code samples may be found in CXX/Sema tests, you just need to
port/simplify them.
================
Comment at: lib/AST/ASTImporter.cpp:3034
@@ +3033,3 @@
+ SmallVector<CXXCtorInitializer *, 4> CtorInitializers;
+ for (const CXXCtorInitializer *I : FromConstructor->inits()) {
+ CXXCtorInitializer *ToI =cast_or_null<CXXCtorInitializer>(
----------------
In my latest patch, I have introduced a function named
`ImportContainerChecked()`. I think it could make this code a bit more clean.
What do you think?
================
Comment at: lib/AST/ASTImporter.cpp:3035
@@ +3034,3 @@
+ for (const CXXCtorInitializer *I : FromConstructor->inits()) {
+ CXXCtorInitializer *ToI =cast_or_null<CXXCtorInitializer>(
+ Importer.Import(I));
----------------
Space after '='
================
Comment at: lib/AST/ASTImporter.cpp:3041
@@ +3040,3 @@
+ }
+ if (unsigned NumInitializers = CtorInitializers.size()) {
+ CXXCtorInitializer **Memory = new (Importer.getToContext())
----------------
If we move this condition (I suggest to use
`FromConstructor->getNumCtorIintializers()` instead) upper to cover the
importing loop, we may skip this loop as well.
================
Comment at: lib/AST/ASTImporter.cpp:3045
@@ +3044,3 @@
+ std::copy(CtorInitializers.begin(), CtorInitializers.end(), Memory);
+ llvm::cast<CXXConstructorDecl>(ToFunction)->setCtorInitializers(Memory);
+ llvm::cast<CXXConstructorDecl>(ToFunction)->setNumCtorInitializers(
----------------
As I can see, LLVM code style avoids qualified `cast`s.
================
Comment at: lib/AST/ASTImporter.cpp:6384
@@ +6383,3 @@
+ return nullptr;
+
+ return new (ToContext)
----------------
Trailing whitespace.
================
Comment at: lib/AST/ASTImporter.cpp:6389
@@ +6388,3 @@
+ Import(From->getRParenLoc()),
+ From->isPackExpansion() ?
+ Import(From->getEllipsisLoc()) : SourceLocation());
----------------
The indentation for `?:` here is a bit confusing.
================
Comment at: lib/AST/ASTImporter.cpp:6396
@@ +6395,3 @@
+ return nullptr;
+
+ return new (ToContext)
----------------
Training whitespace. It's pretty difficult to find them with Phabricator, could
you check your patch for them?
================
Comment at: lib/AST/ASTImporter.cpp:6423
@@ +6422,3 @@
+ } else {
+ return nullptr;
+ }
----------------
It seems like a case with indexed initializer is missed here. You can find my
implementation on
https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L2456-L2464.
A sample that requires it is `auto parens4 = [p4(1)] {};`
(test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p4.cpp)
Repository:
rL LLVM
http://reviews.llvm.org/D20118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
