sepavloff added inline comments. ================ Comment at: lib/AST/ASTImporter.cpp:2373 @@ +2372,3 @@ + Error = true; + ToInfo = TemplateArgumentLocInfo(TSI); + } else { ---------------- Maybe `else` before this statement so that in the case of error `ToInfo` remained default initialized?
================ Comment at: lib/AST/ASTImporter.cpp:2502-2507 @@ +2501,8 @@ + + DeclContext *LexicalDC = DC; + if (D->getDeclContext() != D->getLexicalDeclContext()) { + LexicalDC = Importer.ImportContext(D->getLexicalDeclContext()); + if (!LexicalDC) + return nullptr; + } + ---------------- Is there a reason to handle lexical context of `StaticAssertDecl` separately? I would say `static_assert` is always in the context where it is declared, no? ================ Comment at: lib/AST/ASTImporter.cpp:2518-2519 @@ +2517,4 @@ + Importer.Import(D->getMessage())); + if (!Message) + return nullptr; + ---------------- Since C++17 `static_assert` may be used without message, so null pointer here is not an error. ================ Comment at: lib/AST/ASTImporter.cpp:3356 @@ +3355,3 @@ +Decl *ASTNodeImporter::VisitFriendDecl(FriendDecl *D) { + // Import the major distinguishing characteristics of a variable. + DeclContext *DC = Importer.ImportContext(D->getDeclContext()); ---------------- `variable` -> `declaration`? ================ Comment at: lib/AST/ASTImporter.cpp:3363 @@ +3362,3 @@ + + // Determine whether we've already imported this decl. + // FriendDecl is not a NamedDecl so we cannot use localUncachedLookup. ---------------- Maybe it is more natural to put this check into `bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, RecordDecl *D1, RecordDecl *D2)`? If records are equivalent, existing decl can be used as a result of import, if not - entire record must be recreated anyway. http://reviews.llvm.org/D14326 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits