[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

[Aleksei Sidorin via Phabricator via cfe-commits]

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