[PATCH] D47946: [ASTmporter] Fix infinite recursion on function import with struct definition in parameters

[Gabor Marton via Phabricator via cfe-commits]

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