martong added inline comments.

================
Comment at: clang/lib/AST/ASTImporter.cpp:8448
+
+class AttrImporter {
+  Error Err = Error::success();
----------------
shafik wrote:
> @martong the `ASTImporter` file is quite large as it is and I think it makes 
> sense to have `AttrImporter` but perhaps it would help long-term 
> maintainability to split it out into its own source file? What do you think?
Yes, good idea, I agree. Although, I wouldn't insist to do that in this patch, 
a following patch is okay for me.

After all, it would make sense to split ASTNodeImporter into it's own 
implementation file. Or, even better,  we could put Visit*Type, Visit*Decl and 
Visit*Stmt into separate files. Other than that, the ASTImporterTest.cpp test 
file compiles painfully slow, so that is among my TODOs for a while to split 
that up as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109608

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

Reply via email to