martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.
LGTM, just found some minor things.
================
Comment at: unittests/AST/ASTImporterTest.cpp:174
TranslationUnitDecl *TUDecl = nullptr;
+ std::unique_ptr<ASTImporter> Importer;
+
----------------
Now that the `Importer` is moved under the `TU` struct it is not that trivial
so I think we should add a few more comments to it.
Something like:
```
Represents a "From" translation unit and holds an importer object which we use
to import from this translation unit.
```
Also there is an invariant which should be forced: the "To" and the "From"
context of the `Importer` must remain the same throughout the life time of any
`TU` object. We can achieve that with an assert in `lazyInitImporter`.
(What's more, the different objects all should have the same "To" contexts, but
that could be checked only in the `TestBase` and looks more difficult, so for
this patch we could skip that. )
================
Comment at: unittests/AST/ASTImporterTest.cpp:190
+ Unit->getASTContext(), Unit->getFileManager(), false));
+ }
+ }
----------------
I think, if there is an existing `Importer` then we should assert whether that
has a `ToAST`which is equal to the parameter.
https://reviews.llvm.org/D47946
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
