balazske added inline comments.

================
Comment at: clang/lib/AST/ASTImporter.cpp:3281
 
+  // Import the function parameters.
+  SmallVector<ParmVarDecl *, 8> Parameters;
----------------
shafik wrote:
> I am curious, why move this chunk of code up?
See comments at line 8238?


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5850
+
+TEST_P(ImportTypeLoc, Function) {
+  Decl *FromTU = getTuDecl(
----------------
shafik wrote:
> Maybe I am missing it but these tests don't seem like they cover all the 
> visit methods of `TypeLocImporter`.
It could be useful to add some kind of generic imported AST similarity tests. 
These can test for things that should match in the imported and original AST, 
for example source locations.
(The AST does not always match after import but often.) Such a test can catch 
many import errors with objects that have no specific test.
The test in https://reviews.llvm.org/D60463 is somewhat similar but it uses 
text dump of the AST, this can be improved. Actually that test was used to 
discover the import problems of `SourceLocation` and `TypeSourceInfo`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71018/new/

https://reviews.llvm.org/D71018



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to