a_sidorin added a comment. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411.
Hi Balasz, As I guess, the next step is to convert all `Import`calls to `Import_New` and then rename `Import_New` into `Import`. If so, why aren't we able to just change the behaviour of `Import` methods? Also, there are some comments inline (review is not fully completed, I'll continue review after we agree if this patch is needed). ================ Comment at: include/clang/AST/ASTImporter.h:192 /// - /// \returns the equivalent declaration in the "to" context, or a NULL type - /// if an error occurred. + /// \returns The equivalent declaration in the "to" context, or the import + /// error. ---------------- an import error? ================ Comment at: include/clang/AST/ASTImporter.h:198 + } + // FIXME: Remove this version. Decl *Import(Decl *FromD); ---------------- these versions ================ Comment at: lib/AST/ASTImporter.cpp:7889 + return NestedNameSpecifier::Create(ToContext, Prefix, + Import(FromNNS->getAsIdentifier())); ---------------- We can use Import_New if we change this code, like it is done below. ================ Comment at: lib/AST/ASTImporter.cpp:7922 + return NestedNameSpecifier::Create(ToContext, Prefix, TSTemplate, + (*TyOrErr).getTypePtr()); + } else ---------------- We can use `->` here. Same in some cases below. ================ Comment at: lib/AST/ASTImporter.cpp:7923 + (*TyOrErr).getTypePtr()); + } else + return TyOrErr.takeError(); ---------------- If the true branch is enclosed with braces, the else branch should be enclosed as well. ================ Comment at: lib/AST/ASTImporter.cpp:7938 -NestedNameSpecifierLoc ASTImporter::Import(NestedNameSpecifierLoc FromNNS) { +Expected<NestedNameSpecifierLoc> ASTImporter::Import_New(NestedNameSpecifierLoc FromNNS) { // Copied from NestedNameSpecifier mostly. ---------------- This line exceeds 80-char limit. Repository: rC Clang https://reviews.llvm.org/D53818 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits