[PATCH] D14326: ASTImporter: expressions, pt.2

[Serge Pavlov via cfe-commits]

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