a_sidorin added a comment. Hi Gabor,
To make the review proces faster, you can split the review on separate parts for Stmts, Decls, Types, etc. Otherwise, the review can last for too long. ================ Comment at: lib/AST/ASTImporter.cpp:162 + return llvm::make_error<ImportError>(); + return llvm::Error::success(); + } ---------------- Can we get rid from namespace specifier usages on Error and None? As I see, even in this patch Error is used without it sometimes. ================ Comment at: lib/AST/ASTImporter.cpp:417 - bool ImportDefinition(RecordDecl *From, RecordDecl *To, - ImportDefinitionKind Kind = IDK_Default); - bool ImportDefinition(VarDecl *From, VarDecl *To, - ImportDefinitionKind Kind = IDK_Default); - bool ImportDefinition(EnumDecl *From, EnumDecl *To, - ImportDefinitionKind Kind = IDK_Default); - bool ImportDefinition(ObjCInterfaceDecl *From, ObjCInterfaceDecl *To, - ImportDefinitionKind Kind = IDK_Default); - bool ImportDefinition(ObjCProtocolDecl *From, ObjCProtocolDecl *To, - ImportDefinitionKind Kind = IDK_Default); - TemplateParameterList *ImportTemplateParameterList( + Error ImportDefinition( + RecordDecl *From, RecordDecl *To, ---------------- The changes for argument indentation look redundant. Is it a result of clang-format? ================ Comment at: lib/AST/ASTImporter.cpp:840 +template <> +Expected<TemplateArgumentLoc> +ASTNodeImporter::import(const TemplateArgumentLoc &TALoc) { ---------------- Could you please add some newlines to this function? Its control flow is not trivial so some blocks need to be separated. Repository: rC Clang https://reviews.llvm.org/D51633 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits