[ASTImporter] Use llvm::Expected and Error in the importer API

[martong]

Summary

This is the final phase of the refactoring towards using llvm::Expected
and llvm::Error in the ASTImporter API.
This involves the following:

• remove old Import functions which returned with a pointer,
• use the Import_New functions (which return with Err or Expected) everywhere and handle their return value
• rename Import_New functions to Import

This affects both Clang and LLDB.

[teemperor]

I think the best way to handle these errors in LLDB is to just log and then return some default value. That should make the current command print an error, which is better than terminating LLDB.

Otherwise the LLDB part of this patch LGTM.

[martong]

• Log and do not assert anywhere

[jingham]

IIUC, when Expected returns fails, it returns an Error object that might have information about what went wrong. Would it be possible to include the contents of that error n the log message? We often get "I can't run an expression in a really complex proprietary code base" and need to try to sort out what went wrong from these logs. So every crumb of information we can preserve is useful...

[a_sidorin]

👍

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

277 ↗

(On Diff #197948)

'true' body needs to be surrounded by braces as well. Same below.

[sgraenitz]

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

1980 ↗

(On Diff #197948)

In D61438#1490102, @jingham wrote:

[...] include the contents of that error n the log message?

e.g:

if (auto type_owner = merger.ImporterForOrigin(from_context).Import(type)) {
  return *type_owner;
} else {
  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
  LLDB_LOG_ERROR(log, type_owner.takeError(), "Couldn't import type: {0}")
  return QualType();
}

[martong]

• Add braces to 'true' cases when 'false' case has braces
• Simplify logging and error handling with LLDB_LOG_ERROR

[martong]

• Use LLDB_LOG_ERROR in ImportDefinitionTo

[aprantl]

clang/lib/AST/ASTImporter.cpp

5039 ↗

(On Diff #198623)

We don't typically commit FIXME's into LLVM code. Why not just deal with the error properly from the start?

lldb/source/Symbol/ClangASTImporter.cpp

65 ↗

(On Diff #198623)

if (!delegate_sp)
 return {};

68 ↗

(On Diff #198623)

The else is redundant.

139 ↗

(On Diff #198623)

Can you convert this to an early return instead?

[sgraenitz]

lldb/source/Symbol/ClangASTImporter.cpp

68 ↗

(On Diff #198623)

Here it's necessary for the scope of ret_or_error. That's a bit unfortunate. Maybe invert the condition?

[martong]

clang/lib/AST/ASTImporter.cpp

5039 ↗

(On Diff #198623)

Ok, changed that to return with the error.

lldb/source/Symbol/ClangASTImporter.cpp

68 ↗

(On Diff #198623)

Ok, I have inverted the condition and this way the else became redundant, so I removed it.

139 ↗

(On Diff #198623)

Okay, I have added

if (!delegate_sp)
  return nullptr;

But we can't get rid of consumeError because otherwise the error object remains unchecked and will cause run-time assert.
LLDB_LOG_ERROR calls consumeError too plus logs the error msg, so I changed to use that.

[martong]

• Remove FIXME and return the error
• Use early return where possible and remove redundant else

[martong]

• Remove remaining FIXMEs

[aprantl]

clang/include/clang/AST/ASTImporter.h

203 ↗

(On Diff #198824)

Wouldn't it make more sense to return Expected<TypeSourceInfo &>?

[martong]

clang/include/clang/AST/ASTImporter.h

203 ↗

(On Diff #198824)

That would not be consistent with the other changes. In this patch we change the signature of each functions similarly:
From T foo(...) to Expected<T> foo(...).
Also, TypeSourceInfo factory functions in ASTContext do return with a pointer. For example:

TypeSourceInfo *CreateTypeSourceInfo(QualType T, unsigned Size = 0) const;

/// Allocate a TypeSourceInfo where all locations have been
/// initialized to a given location, which defaults to the empty
/// location.
TypeSourceInfo *
getTrivialTypeSourceInfo(QualType T,
                         SourceLocation Loc = SourceLocation()) const;

[martong]

@aprantl Ping

[aprantl]

clang/include/clang/AST/ASTImporter.h

203 ↗

(On Diff #198824)

I believe that LLVM's practice of passing non-null pointers around is bad API design because it's never quite clear from looking at an API which pointer parameters are nullable, so I was hoping that we could correct some of that at least in the cases where we introduce API that return Expected<> objects to make it obvious that this is either an error or a valid object. If you that the perceived inconsistency weighs stronger than the readability improvements let me know and I can reconsider.

[martong]

clang/include/clang/AST/ASTImporter.h

203 ↗

(On Diff #198824)

I couldn't agree more that having non-null-able pointers is a bad API, and we'd be better to have references in these cases. However, I think the situation is more complex than that.

If we changed TypeSourceInfo * to TypeSourceInfo & in this patch, that would involve to make similar changes with other importer functions (NestedNameSpecifier *, Decl *, Expr *, etc.). That would result in a huge patch and I am afraid we could not follow the incremental development suggestion in the LLVM dev policy. Thus, even if we decide to change to references I propose to do that in a separate patch.

Also, some AST nodes are indeed null-able. E.g. the body of a FunctionDecl might be null. Or the described class template of a CXXRecordDecl may be null (and we rely on this, because first we create the CXXRecordDecl then we import the ClassTemplateDecl and only then we set the relation).
The point is that we should identify those AST nodes which are really non-nullable in the AST. This does not seem trivial.
Besides, since the ASTImporter class is part of the AST lib, it would look strange if we changed to use references only in the ASTImporter. Perhaps, we should change in the whole AST API, but that might broke too many dependencies I guess. Still, this may worth an RFC on cfe-dev.

[aprantl]

Alright, thanks for taking the time to walk me through the thought process!

[martong]

Thank you guys for the review!

In D61438#1501457, @aprantl wrote:

Alright, thanks for taking the time to walk me through the thought process!

[martong]

• Rebase to master

cfe/trunk/include/clang/AST/ASTImporter.h

public:
 
     /// \brief Import the given object, returns the result.
     ///
     /// \param To Import the object into this variable.
     /// \param From Object to import.
     /// \return Error information (success or error).
     template <typename ImportT>
     LLVM_NODISCARD llvm::Error importInto(ImportT &To, const ImportT &From) {
-      auto ToOrErr = Import_New(From);
+      auto ToOrErr = Import(From);
       if (ToOrErr)
         To = *ToOrErr;
       return ToOrErr.takeError();
     }
 
     /// Import the given type from the "from" context into the "to"
     /// context. A null type is imported as a null type (no error).
     ///
     /// \returns The equivalent type in the "to" context, or the import error.
-    llvm::Expected<QualType> Import_New(QualType FromT);
-    // FIXME: Remove this version.
-    QualType Import(QualType FromT);
+    llvm::Expected<QualType> Import(QualType FromT);
 
     /// Import the given type source information from the
     /// "from" context into the "to" context.
     ///
     /// \returns The equivalent type source information in the "to"
     /// context, or the import error.
-    llvm::Expected<TypeSourceInfo *> Import_New(TypeSourceInfo *FromTSI);
-    // FIXME: Remove this version.
-    TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
+    llvm::Expected<TypeSourceInfo *> Import(TypeSourceInfo *FromTSI);
 
     /// Import the given attribute from the "from" context into the
     /// "to" context.
     ///
     /// \returns The equivalent attribute in the "to" context, or the import
     /// error.
-    llvm::Expected<Attr *> Import_New(const Attr *FromAttr);
-    // FIXME: Remove this version.
-    Attr *Import(const Attr *FromAttr);
+    llvm::Expected<Attr *> Import(const Attr *FromAttr);
 
     /// Import the given declaration from the "from" context into the
     /// "to" context.
     ///
     /// \returns The equivalent declaration in the "to" context, or the import
     /// error.
-    llvm::Expected<Decl *> Import_New(Decl *FromD);
-    llvm::Expected<const Decl *> Import_New(const Decl *FromD) {
-      return Import_New(const_cast<Decl *>(FromD));
-    }
-    // FIXME: Remove this version.
-    Decl *Import(Decl *FromD);
-    Decl *Import(const Decl *FromD) {
+    llvm::Expected<Decl *> Import(Decl *FromD);
+    llvm::Expected<const Decl *> Import(const Decl *FromD) {
       return Import(const_cast<Decl *>(FromD));
     }
 
     /// Return the copy of the given declaration in the "to" context if
     /// it has already been imported from the "from" context.  Otherwise return
     /// nullptr.
     Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
 
     /// Return the translation unit from where the declaration was
     /// imported. If it does not exist nullptr is returned.
     TranslationUnitDecl *GetFromTU(Decl *ToD);
 
     /// Import the given declaration context from the "from"
     /// AST context into the "to" AST context.
     ///
     /// \returns the equivalent declaration context in the "to"
     /// context, or error value.
     llvm::Expected<DeclContext *> ImportContext(DeclContext *FromDC);
 
     /// Import the given expression from the "from" context into the
     /// "to" context.
     ///
     /// \returns The equivalent expression in the "to" context, or the import
     /// error.
-    llvm::Expected<Expr *> Import_New(Expr *FromE);
-    // FIXME: Remove this version.
-    Expr *Import(Expr *FromE);
+    llvm::Expected<Expr *> Import(Expr *FromE);
 
     /// Import the given statement from the "from" context into the
     /// "to" context.
     ///
     /// \returns The equivalent statement in the "to" context, or the import
     /// error.
-    llvm::Expected<Stmt *> Import_New(Stmt *FromS);
-    // FIXME: Remove this version.
-    Stmt *Import(Stmt *FromS);
+    llvm::Expected<Stmt *> Import(Stmt *FromS);
 
     /// Import the given nested-name-specifier from the "from"
     /// context into the "to" context.
     ///
     /// \returns The equivalent nested-name-specifier in the "to"
     /// context, or the import error.
-    llvm::Expected<NestedNameSpecifier *>
-    Import_New(NestedNameSpecifier *FromNNS);
-    // FIXME: Remove this version.
-    NestedNameSpecifier *Import(NestedNameSpecifier *FromNNS);
+    llvm::Expected<NestedNameSpecifier *> Import(NestedNameSpecifier *FromNNS);
 
     /// Import the given nested-name-specifier-loc from the "from"
     /// context into the "to" context.
     ///
     /// \returns The equivalent nested-name-specifier-loc in the "to"
     /// context, or the import error.
     llvm::Expected<NestedNameSpecifierLoc>
-    Import_New(NestedNameSpecifierLoc FromNNS);
-    // FIXME: Remove this version.
-    NestedNameSpecifierLoc Import(NestedNameSpecifierLoc FromNNS);
+    Import(NestedNameSpecifierLoc FromNNS);
 
     /// Import the given template name from the "from" context into the
     /// "to" context, or the import error.
-    llvm::Expected<TemplateName> Import_New(TemplateName From);
-    // FIXME: Remove this version.
-    TemplateName Import(TemplateName From);
+    llvm::Expected<TemplateName> Import(TemplateName From);
 
     /// Import the given source location from the "from" context into
     /// the "to" context.
     ///
     /// \returns The equivalent source location in the "to" context, or the
     /// import error.
-    llvm::Expected<SourceLocation> Import_New(SourceLocation FromLoc);
-    // FIXME: Remove this version.
-    SourceLocation Import(SourceLocation FromLoc);
+    llvm::Expected<SourceLocation> Import(SourceLocation FromLoc);
 
     /// Import the given source range from the "from" context into
     /// the "to" context.
     ///
     /// \returns The equivalent source range in the "to" context, or the import
     /// error.
-    llvm::Expected<SourceRange> Import_New(SourceRange FromRange);
-    // FIXME: Remove this version.
-    SourceRange Import(SourceRange FromRange);
+    llvm::Expected<SourceRange> Import(SourceRange FromRange);
 
     /// Import the given declaration name from the "from"
     /// context into the "to" context.
     ///
     /// \returns The equivalent declaration name in the "to" context, or the
     /// import error.
-    llvm::Expected<DeclarationName> Import_New(DeclarationName FromName);
-    // FIXME: Remove this version.
-    DeclarationName Import(DeclarationName FromName);
+    llvm::Expected<DeclarationName> Import(DeclarationName FromName);
 
     /// Import the given identifier from the "from" context
     /// into the "to" context.
     ///
     /// \returns The equivalent identifier in the "to" context. Note: It
     /// returns nullptr only if the FromId was nullptr.
     IdentifierInfo *Import(const IdentifierInfo *FromId);
 
     /// Import the given Objective-C selector from the "from"
     /// context into the "to" context.
     ///
     /// \returns The equivalent selector in the "to" context, or the import
     /// error.
-    llvm::Expected<Selector> Import_New(Selector FromSel);
-    // FIXME: Remove this version.
-    Selector Import(Selector FromSel);
+    llvm::Expected<Selector> Import(Selector FromSel);
 
     /// Import the given file ID from the "from" context into the
     /// "to" context.
     ///
     /// \returns The equivalent file ID in the source manager of the "to"
     /// context, or the import error.
-    llvm::Expected<FileID> Import_New(FileID, bool IsBuiltin = false);
-    // FIXME: Remove this version.
-    FileID Import(FileID, bool IsBuiltin = false);
+    llvm::Expected<FileID> Import(FileID, bool IsBuiltin = false);
 
     /// Import the given C++ constructor initializer from the "from"
     /// context into the "to" context.
     ///
     /// \returns The equivalent initializer in the "to" context, or the import
     /// error.
-    llvm::Expected<CXXCtorInitializer *>
-    Import_New(CXXCtorInitializer *FromInit);
-    // FIXME: Remove this version.
-    CXXCtorInitializer *Import(CXXCtorInitializer *FromInit);
+    llvm::Expected<CXXCtorInitializer *> Import(CXXCtorInitializer *FromInit);
 
     /// Import the given CXXBaseSpecifier from the "from" context into
     /// the "to" context.
     ///
     /// \returns The equivalent CXXBaseSpecifier in the source manager of the
     /// "to" context, or the import error.
-    llvm::Expected<CXXBaseSpecifier *>
-    Import_New(const CXXBaseSpecifier *FromSpec);
-    // FIXME: Remove this version.
-    CXXBaseSpecifier *Import(const CXXBaseSpecifier *FromSpec);
+    llvm::Expected<CXXBaseSpecifier *> Import(const CXXBaseSpecifier *FromSpec);
 
     /// Import the definition of the given declaration, including all of
     /// the declarations it contains.
-    LLVM_NODISCARD llvm::Error ImportDefinition_New(Decl *From);
-
-    // FIXME: Compatibility function.
-    // Usages of this should be changed to ImportDefinition_New.
-    void ImportDefinition(Decl *From);
+    LLVM_NODISCARD llvm::Error ImportDefinition(Decl *From);
 
     /// Cope with a name conflict when importing a declaration into the
     /// given context.
     ///
     /// This routine is invoked whenever there is a name conflict while
     /// importing a declaration. The returned name will become the name of the
     /// imported declaration. By default, the returned name is the same as the
     /// original name, leaving the conflict unresolve such that name lookup

cfe/trunk/lib/AST/ASTImporter.cpp

class ASTNodeImporter : public TypeVisitor<ASTNodeImporter, ExpectedType>,
     template <typename ImportT>
     LLVM_NODISCARD Error importInto(ImportT &To, const ImportT &From) {
       return Importer.importInto(To, From);
     }
 
     // Use this to import pointers of specific type.
     template <typename ImportT>
     LLVM_NODISCARD Error importInto(ImportT *&To, ImportT *From) {
-      auto ToOrErr = Importer.Import_New(From);
+      auto ToOrErr = Importer.Import(From);
       if (ToOrErr)
         To = cast_or_null<ImportT>(*ToOrErr);
       return ToOrErr.takeError();
     }
 
     // Call the import function of ASTImporter for a baseclass of type `T` and
     // cast the return value to `T`.
     template <typename T>
     Expected<T *> import(T *From) {
-      auto ToOrErr = Importer.Import_New(From);
+      auto ToOrErr = Importer.Import(From);
       if (!ToOrErr)
         return ToOrErr.takeError();
       return cast_or_null<T>(*ToOrErr);
     }
 
     template <typename T>
     Expected<T *> import(const T *From) {
       return import(const_cast<T *>(From));
     }
 
     // Call the import function of ASTImporter for type `T`.
     template <typename T>
     Expected<T> import(const T &From) {
-      return Importer.Import_New(From);
+      return Importer.Import(From);
     }
 
     // Import an Optional<T> by importing the contained T, if any.
     template<typename T>
     Expected<Optional<T>> import(Optional<T> From) {
       if (!From)
         return Optional<T>();
       return import(*From);
for (CXXMethodDecl *FromM : From->methods())
     }
 
   return Error::success();
 }
 
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
                                        ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-    if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+    if (ExpectedDecl ToTypedefOrErr = Importer.Import(FromTypedef))
       To->setTypedefNameForAnonDecl(cast<TypedefNameDecl>(*ToTypedefOrErr));
     else
       return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
 
 Error ASTNodeImporter::ImportDefinition(
else
     PrevDecl = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
 
   if (PrevDecl) {
     if (IsStructuralMatch(D, PrevDecl)) {
       if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) {
         Importer.MapImported(D, PrevDecl->getDefinition());
         // Import those default field initializers which have been
         // instantiated in the "From" context, but not in the "To" context.
-        for (auto *FromField : D->fields())
-          Importer.Import(FromField);
+        for (auto *FromField : D->fields()) {
+          auto ToOrErr = import(FromField);
+          if (!ToOrErr)
+            return ToOrErr.takeError();
+        }
 
         // Import those methods which have been instantiated in the
         // "From" context, but not in the "To" context.
-        for (CXXMethodDecl *FromM : D->methods())
-          Importer.Import(FromM);
+        for (CXXMethodDecl *FromM : D->methods()) {
+          auto ToOrErr = import(FromM);
+          if (!ToOrErr)
+            return ToOrErr.takeError();
+        }
 
         // TODO Import instantiated default arguments.
         // TODO Import instantiated exception specifications.
         //
         // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint
         // what else could be fused during an AST merge.
         return PrevDecl;
       }
Expected<Decl *> ASTImporter::ImportImpl(Decl *FromD) {
   return Importer.Visit(FromD);
 }
 
 void ASTImporter::RegisterImportedDecl(Decl *FromD, Decl *ToD) {
   MapImported(FromD, ToD);
   AddToLookupTable(ToD);
 }
 
-Expected<QualType> ASTImporter::Import_New(QualType FromT) {
+Expected<QualType> ASTImporter::Import(QualType FromT) {
   if (FromT.isNull())
     return QualType{};
 
   const Type *FromTy = FromT.getTypePtr();
 
   // Check whether we've already imported this type.
   llvm::DenseMap<const Type *, const Type *>::iterator Pos
     = ImportedTypes.find(FromTy);
   if (Pos != ImportedTypes.end())
     return ToContext.getQualifiedType(Pos->second, FromT.getLocalQualifiers());
 
   // Import the type
   ASTNodeImporter Importer(*this);
   ExpectedType ToTOrErr = Importer.Visit(FromTy);
   if (!ToTOrErr)
     return ToTOrErr.takeError();
 
   // Record the imported type.
   ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
 
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
-QualType ASTImporter::Import(QualType From) {
-  llvm::Expected<QualType> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<TypeSourceInfo *> ASTImporter::Import_New(TypeSourceInfo *FromTSI) {
+Expected<TypeSourceInfo *> ASTImporter::Import(TypeSourceInfo *FromTSI) {
   if (!FromTSI)
     return FromTSI;
 
   // FIXME: For now we just create a "trivial" type source info based
   // on the type and a single location. Implement a real version of this.
-  ExpectedType TOrErr = Import_New(FromTSI->getType());
+  ExpectedType TOrErr = Import(FromTSI->getType());
   if (!TOrErr)
     return TOrErr.takeError();
-  ExpectedSLoc BeginLocOrErr = Import_New(FromTSI->getTypeLoc().getBeginLoc());
+  ExpectedSLoc BeginLocOrErr = Import(FromTSI->getTypeLoc().getBeginLoc());
   if (!BeginLocOrErr)
     return BeginLocOrErr.takeError();
 
   return ToContext.getTrivialTypeSourceInfo(*TOrErr, *BeginLocOrErr);
 }
-TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *From) {
-  llvm::Expected<TypeSourceInfo *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
-Expected<Attr *> ASTImporter::Import_New(const Attr *FromAttr) {
+Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
   Attr *ToAttr = FromAttr->clone(ToContext);
-  if (auto ToRangeOrErr = Import_New(FromAttr->getRange()))
+  if (auto ToRangeOrErr = Import(FromAttr->getRange()))
     ToAttr->setRange(*ToRangeOrErr);
   else
     return ToRangeOrErr.takeError();
 
   return ToAttr;
 }
-Attr *ASTImporter::Import(const Attr *From) {
-  llvm::Expected<Attr *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
 Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
   auto Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end())
     return Pos->second;
   else
     return nullptr;
 }
 
 TranslationUnitDecl *ASTImporter::GetFromTU(Decl *ToD) {
   auto FromDPos = ImportedFromDecls.find(ToD);
   if (FromDPos == ImportedFromDecls.end())
     return nullptr;
   return FromDPos->second->getTranslationUnitDecl();
 }
 
-Expected<Decl *> ASTImporter::Import_New(Decl *FromD) {
+Expected<Decl *> ASTImporter::Import(Decl *FromD) {
   if (!FromD)
     return nullptr;
 
 
   // Check whether we've already imported this declaration.
   Decl *ToD = GetAlreadyImportedOrNull(FromD);
   if (ToD) {
     // If FromD has some updated flags after last import, apply it
Expected<Decl *> ASTImporter::Import(Decl *FromD) {
   AddToLookupTable(ToD);
 
   // Notify subclasses.
   Imported(FromD, ToD);
 
   updateFlags(FromD, ToD);
   return ToDOrErr;
 }
-Decl *ASTImporter::Import(Decl *From) {
-  llvm::Expected<Decl *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
 Expected<DeclContext *> ASTImporter::ImportContext(DeclContext *FromDC) {
   if (!FromDC)
     return FromDC;
 
-  ExpectedDecl ToDCOrErr = Import_New(cast<Decl>(FromDC));
+  ExpectedDecl ToDCOrErr = Import(cast<Decl>(FromDC));
   if (!ToDCOrErr)
     return ToDCOrErr.takeError();
   auto *ToDC = cast<DeclContext>(*ToDCOrErr);
 
   // When we're using a record/enum/Objective-C class/protocol as a context, we
   // need it to have a definition.
   if (auto *ToRecord = dyn_cast<RecordDecl>(ToDC)) {
     auto *FromRecord = cast<RecordDecl>(FromDC);
if (auto *ToRecord = dyn_cast<RecordDecl>(ToDC)) {
     } else {
       CompleteDecl(ToProto);
     }
   }
 
   return ToDC;
 }
 
-Expected<Expr *> ASTImporter::Import_New(Expr *FromE) {
-  if (ExpectedStmt ToSOrErr = Import_New(cast_or_null<Stmt>(FromE)))
+Expected<Expr *> ASTImporter::Import(Expr *FromE) {
+  if (ExpectedStmt ToSOrErr = Import(cast_or_null<Stmt>(FromE)))
     return cast_or_null<Expr>(*ToSOrErr);
   else
     return ToSOrErr.takeError();
 }
-Expr *ASTImporter::Import(Expr *From) {
-  llvm::Expected<Expr *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
-Expected<Stmt *> ASTImporter::Import_New(Stmt *FromS) {
+Expected<Stmt *> ASTImporter::Import(Stmt *FromS) {
   if (!FromS)
     return nullptr;
 
   // Check whether we've already imported this statement.
   llvm::DenseMap<Stmt *, Stmt *>::iterator Pos = ImportedStmts.find(FromS);
   if (Pos != ImportedStmts.end())
     return Pos->second;
 
if (auto *ToE = dyn_cast<Expr>(*ToSOrErr)) {
     ToE->setContainsUnexpandedParameterPack(
         FromE->containsUnexpandedParameterPack());
   }
 
   // Record the imported statement object.
   ImportedStmts[FromS] = *ToSOrErr;
   return ToSOrErr;
 }
-Stmt *ASTImporter::Import(Stmt *From) {
-  llvm::Expected<Stmt *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
 Expected<NestedNameSpecifier *>
-ASTImporter::Import_New(NestedNameSpecifier *FromNNS) {
+ASTImporter::Import(NestedNameSpecifier *FromNNS) {
   if (!FromNNS)
     return nullptr;
 
   NestedNameSpecifier *Prefix;
   if (Error Err = importInto(Prefix, FromNNS->getPrefix()))
     return std::move(Err);
 
   switch (FromNNS->getKind()) {
   case NestedNameSpecifier::Identifier:
     assert(FromNNS->getAsIdentifier() && "NNS should contain identifier.");
     return NestedNameSpecifier::Create(ToContext, Prefix,
                                        Import(FromNNS->getAsIdentifier()));
 
   case NestedNameSpecifier::Namespace:
-    if (ExpectedDecl NSOrErr = Import_New(FromNNS->getAsNamespace())) {
+    if (ExpectedDecl NSOrErr = Import(FromNNS->getAsNamespace())) {
       return NestedNameSpecifier::Create(ToContext, Prefix,
                                          cast<NamespaceDecl>(*NSOrErr));
     } else
       return NSOrErr.takeError();
 
   case NestedNameSpecifier::NamespaceAlias:
-    if (ExpectedDecl NSADOrErr = Import_New(FromNNS->getAsNamespaceAlias()))
+    if (ExpectedDecl NSADOrErr = Import(FromNNS->getAsNamespaceAlias()))
       return NestedNameSpecifier::Create(ToContext, Prefix,
                                          cast<NamespaceAliasDecl>(*NSADOrErr));
     else
       return NSADOrErr.takeError();
 
   case NestedNameSpecifier::Global:
     return NestedNameSpecifier::GlobalSpecifier(ToContext);
 
   case NestedNameSpecifier::Super:
-    if (ExpectedDecl RDOrErr = Import_New(FromNNS->getAsRecordDecl()))
+    if (ExpectedDecl RDOrErr = Import(FromNNS->getAsRecordDecl()))
       return NestedNameSpecifier::SuperSpecifier(ToContext,
                                                  cast<CXXRecordDecl>(*RDOrErr));
     else
       return RDOrErr.takeError();
 
   case NestedNameSpecifier::TypeSpec:
   case NestedNameSpecifier::TypeSpecWithTemplate:
     if (Expected<QualType> TyOrErr =
-            Import_New(QualType(FromNNS->getAsType(), 0u))) {
+            Import(QualType(FromNNS->getAsType(), 0u))) {
       bool TSTemplate =
           FromNNS->getKind() == NestedNameSpecifier::TypeSpecWithTemplate;
       return NestedNameSpecifier::Create(ToContext, Prefix, TSTemplate,
                                          TyOrErr->getTypePtr());
     } else {
       return TyOrErr.takeError();
     }
   }
 
   llvm_unreachable("Invalid nested name specifier kind");
 }
-NestedNameSpecifier *ASTImporter::Import(NestedNameSpecifier *From) {
-  llvm::Expected<NestedNameSpecifier *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
 Expected<NestedNameSpecifierLoc>
-ASTImporter::Import_New(NestedNameSpecifierLoc FromNNS) {
+ASTImporter::Import(NestedNameSpecifierLoc FromNNS) {
   // Copied from NestedNameSpecifier mostly.
   SmallVector<NestedNameSpecifierLoc , 8> NestedNames;
   NestedNameSpecifierLoc NNS = FromNNS;
 
   // Push each of the nested-name-specifiers's onto a stack for
   // serialization in reverse order.
   while (NNS) {
     NestedNames.push_back(NNS);
case NestedNameSpecifier::TypeSpecWithTemplate: {
       break;
     }
 
     case NestedNameSpecifier::Global:
       Builder.MakeGlobal(getToContext(), ToLocalBeginLoc);
       break;
 
     case NestedNameSpecifier::Super: {
-      auto ToSourceRangeOrErr = Import_New(NNS.getSourceRange());
+      auto ToSourceRangeOrErr = Import(NNS.getSourceRange());
       if (!ToSourceRangeOrErr)
         return ToSourceRangeOrErr.takeError();
 
       Builder.MakeSuper(getToContext(), Spec->getAsRecordDecl(),
                         ToSourceRangeOrErr->getBegin(),
                         ToSourceRangeOrErr->getEnd());
     }
   }
   }
 
   return Builder.getWithLocInContext(getToContext());
 }
-NestedNameSpecifierLoc ASTImporter::Import(NestedNameSpecifierLoc From) {
-  llvm::Expected<NestedNameSpecifierLoc> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<TemplateName> ASTImporter::Import_New(TemplateName From) {
+Expected<TemplateName> ASTImporter::Import(TemplateName From) {
   switch (From.getKind()) {
   case TemplateName::Template:
-    if (ExpectedDecl ToTemplateOrErr = Import_New(From.getAsTemplateDecl()))
+    if (ExpectedDecl ToTemplateOrErr = Import(From.getAsTemplateDecl()))
       return TemplateName(cast<TemplateDecl>(*ToTemplateOrErr));
     else
       return ToTemplateOrErr.takeError();
 
   case TemplateName::OverloadedTemplate: {
     OverloadedTemplateStorage *FromStorage = From.getAsOverloadedTemplate();
     UnresolvedSet<2> ToTemplates;
     for (auto *I : *FromStorage) {
-      if (auto ToOrErr = Import_New(I))
+      if (auto ToOrErr = Import(I))
         ToTemplates.addDecl(cast<NamedDecl>(*ToOrErr));
       else
         return ToOrErr.takeError();
     }
     return ToContext.getOverloadedTemplateName(ToTemplates.begin(),
                                                ToTemplates.end());
   }
 
   case TemplateName::AssumedTemplate: {
     AssumedTemplateStorage *FromStorage = From.getAsAssumedTemplateName();
-    auto DeclNameOrErr = Import_New(FromStorage->getDeclName());
+    auto DeclNameOrErr = Import(FromStorage->getDeclName());
     if (!DeclNameOrErr)
       return DeclNameOrErr.takeError();
     return ToContext.getAssumedTemplateName(*DeclNameOrErr);
   }
 
   case TemplateName::QualifiedTemplate: {
     QualifiedTemplateName *QTN = From.getAsQualifiedTemplateName();
-    auto QualifierOrErr = Import_New(QTN->getQualifier());
+    auto QualifierOrErr = Import(QTN->getQualifier());
     if (!QualifierOrErr)
       return QualifierOrErr.takeError();
 
-    if (ExpectedDecl ToTemplateOrErr = Import_New(From.getAsTemplateDecl()))
+    if (ExpectedDecl ToTemplateOrErr = Import(From.getAsTemplateDecl()))
       return ToContext.getQualifiedTemplateName(
           *QualifierOrErr, QTN->hasTemplateKeyword(),
           cast<TemplateDecl>(*ToTemplateOrErr));
     else
       return ToTemplateOrErr.takeError();
   }
 
   case TemplateName::DependentTemplate: {
     DependentTemplateName *DTN = From.getAsDependentTemplateName();
-    auto QualifierOrErr = Import_New(DTN->getQualifier());
+    auto QualifierOrErr = Import(DTN->getQualifier());
     if (!QualifierOrErr)
       return QualifierOrErr.takeError();
 
     if (DTN->isIdentifier()) {
       return ToContext.getDependentTemplateName(*QualifierOrErr,
                                                 Import(DTN->getIdentifier()));
     }
 
     return ToContext.getDependentTemplateName(*QualifierOrErr,
                                               DTN->getOperator());
   }
 
   case TemplateName::SubstTemplateTemplateParm: {
     SubstTemplateTemplateParmStorage *Subst =
         From.getAsSubstTemplateTemplateParm();
-    ExpectedDecl ParamOrErr = Import_New(Subst->getParameter());
+    ExpectedDecl ParamOrErr = Import(Subst->getParameter());
     if (!ParamOrErr)
       return ParamOrErr.takeError();
 
-    auto ReplacementOrErr = Import_New(Subst->getReplacement());
+    auto ReplacementOrErr = Import(Subst->getReplacement());
     if (!ReplacementOrErr)
       return ReplacementOrErr.takeError();
 
     return ToContext.getSubstTemplateTemplateParm(
         cast<TemplateTemplateParmDecl>(*ParamOrErr), *ReplacementOrErr);
   }
 
   case TemplateName::SubstTemplateTemplateParmPack: {
     SubstTemplateTemplateParmPackStorage *SubstPack
       = From.getAsSubstTemplateTemplateParmPack();
-    ExpectedDecl ParamOrErr = Import_New(SubstPack->getParameterPack());
+    ExpectedDecl ParamOrErr = Import(SubstPack->getParameterPack());
     if (!ParamOrErr)
       return ParamOrErr.takeError();
 
     ASTNodeImporter Importer(*this);
     auto ArgPackOrErr =
         Importer.ImportTemplateArgument(SubstPack->getArgumentPack());
     if (!ArgPackOrErr)
       return ArgPackOrErr.takeError();
 
     return ToContext.getSubstTemplateTemplateParmPack(
         cast<TemplateTemplateParmDecl>(*ParamOrErr), *ArgPackOrErr);
   }
   }
 
   llvm_unreachable("Invalid template name kind");
 }
-TemplateName ASTImporter::Import(TemplateName From) {
-  llvm::Expected<TemplateName> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<SourceLocation> ASTImporter::Import_New(SourceLocation FromLoc) {
+Expected<SourceLocation> ASTImporter::Import(SourceLocation FromLoc) {
   if (FromLoc.isInvalid())
     return SourceLocation{};
 
   SourceManager &FromSM = FromContext.getSourceManager();
   bool IsBuiltin = FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair<FileID, unsigned> Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  Expected<FileID> ToFileIDOrErr = Import_New(Decomposed.first, IsBuiltin);
+  Expected<FileID> ToFileIDOrErr = Import(Decomposed.first, IsBuiltin);
   if (!ToFileIDOrErr)
     return ToFileIDOrErr.takeError();
   SourceManager &ToSM = ToContext.getSourceManager();
   return ToSM.getComposedLoc(*ToFileIDOrErr, Decomposed.second);
 }
-SourceLocation ASTImporter::Import(SourceLocation From) {
-  llvm::Expected<SourceLocation> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<SourceRange> ASTImporter::Import_New(SourceRange FromRange) {
+Expected<SourceRange> ASTImporter::Import(SourceRange FromRange) {
   SourceLocation ToBegin, ToEnd;
   if (Error Err = importInto(ToBegin, FromRange.getBegin()))
     return std::move(Err);
   if (Error Err = importInto(ToEnd, FromRange.getEnd()))
     return std::move(Err);
 
   return SourceRange(ToBegin, ToEnd);
 }
-SourceRange ASTImporter::Import(SourceRange From) {
-  llvm::Expected<SourceRange> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<FileID> ASTImporter::Import_New(FileID FromID, bool IsBuiltin) {
+Expected<FileID> ASTImporter::Import(FileID FromID, bool IsBuiltin) {
   llvm::DenseMap<FileID, FileID>::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
     return Pos->second;
 
   SourceManager &FromSM = FromContext.getSourceManager();
   SourceManager &ToSM = ToContext.getSourceManager();
   const SrcMgr::SLocEntry &FromSLoc = FromSM.getSLocEntry(FromID);
 
   // Map the FromID to the "to" source manager.
   FileID ToID;
   if (FromSLoc.isExpansion()) {
     const SrcMgr::ExpansionInfo &FromEx = FromSLoc.getExpansion();
-    ExpectedSLoc ToSpLoc = Import_New(FromEx.getSpellingLoc());
+    ExpectedSLoc ToSpLoc = Import(FromEx.getSpellingLoc());
     if (!ToSpLoc)
       return ToSpLoc.takeError();
-    ExpectedSLoc ToExLocS = Import_New(FromEx.getExpansionLocStart());
+    ExpectedSLoc ToExLocS = Import(FromEx.getExpansionLocStart());
     if (!ToExLocS)
       return ToExLocS.takeError();
     unsigned TokenLen = FromSM.getFileIDSize(FromID);
     SourceLocation MLoc;
     if (FromEx.isMacroArgExpansion()) {
       MLoc = ToSM.createMacroArgExpansionLoc(*ToSpLoc, *ToExLocS, TokenLen);
     } else {
-      if (ExpectedSLoc ToExLocE = Import_New(FromEx.getExpansionLocEnd()))
+      if (ExpectedSLoc ToExLocE = Import(FromEx.getExpansionLocEnd()))
         MLoc = ToSM.createExpansionLoc(*ToSpLoc, *ToExLocS, *ToExLocE, TokenLen,
                                        FromEx.isExpansionTokenRange());
       else
         return ToExLocE.takeError();
     }
     ToID = ToSM.getFileID(MLoc);
   } else {
     const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
 
     if (!IsBuiltin) {
       // Include location of this file.
-      ExpectedSLoc ToIncludeLoc =
-          Import_New(FromSLoc.getFile().getIncludeLoc());
+      ExpectedSLoc ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
       if (!ToIncludeLoc)
         return ToIncludeLoc.takeError();
 
       if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
         // FIXME: We probably want to use getVirtualFile(), so we don't hit the
         // disk again
         // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
         // than mmap the files several times.
if (FromSLoc.isExpansion()) {
     }
   }
 
   assert(ToID.isValid() && "Unexpected invalid fileID was created.");
 
   ImportedFileIDs[FromID] = ToID;
   return ToID;
 }
-FileID ASTImporter::Import(FileID From, bool IsBuiltin) {
-  llvm::Expected<FileID> To = Import_New(From, IsBuiltin);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
-Expected<CXXCtorInitializer *>
-ASTImporter::Import_New(CXXCtorInitializer *From) {
-  ExpectedExpr ToExprOrErr = Import_New(From->getInit());
+Expected<CXXCtorInitializer *> ASTImporter::Import(CXXCtorInitializer *From) {
+  ExpectedExpr ToExprOrErr = Import(From->getInit());
   if (!ToExprOrErr)
     return ToExprOrErr.takeError();
 
-  auto LParenLocOrErr = Import_New(From->getLParenLoc());
+  auto LParenLocOrErr = Import(From->getLParenLoc());
   if (!LParenLocOrErr)
     return LParenLocOrErr.takeError();
 
-  auto RParenLocOrErr = Import_New(From->getRParenLoc());
+  auto RParenLocOrErr = Import(From->getRParenLoc());
   if (!RParenLocOrErr)
     return RParenLocOrErr.takeError();
 
   if (From->isBaseInitializer()) {
-    auto ToTInfoOrErr = Import_New(From->getTypeSourceInfo());
+    auto ToTInfoOrErr = Import(From->getTypeSourceInfo());
     if (!ToTInfoOrErr)
       return ToTInfoOrErr.takeError();
 
     SourceLocation EllipsisLoc;
     if (From->isPackExpansion())
       if (Error Err = importInto(EllipsisLoc, From->getEllipsisLoc()))
         return std::move(Err);
 
     return new (ToContext) CXXCtorInitializer(
         ToContext, *ToTInfoOrErr, From->isBaseVirtual(), *LParenLocOrErr,
         *ToExprOrErr, *RParenLocOrErr, EllipsisLoc);
   } else if (From->isMemberInitializer()) {
-    ExpectedDecl ToFieldOrErr = Import_New(From->getMember());
+    ExpectedDecl ToFieldOrErr = Import(From->getMember());
     if (!ToFieldOrErr)
       return ToFieldOrErr.takeError();
 
-    auto MemberLocOrErr = Import_New(From->getMemberLocation());
+    auto MemberLocOrErr = Import(From->getMemberLocation());
     if (!MemberLocOrErr)
       return MemberLocOrErr.takeError();
 
     return new (ToContext) CXXCtorInitializer(
         ToContext, cast_or_null<FieldDecl>(*ToFieldOrErr), *MemberLocOrErr,
         *LParenLocOrErr, *ToExprOrErr, *RParenLocOrErr);
   } else if (From->isIndirectMemberInitializer()) {
-    ExpectedDecl ToIFieldOrErr = Import_New(From->getIndirectMember());
+    ExpectedDecl ToIFieldOrErr = Import(From->getIndirectMember());
     if (!ToIFieldOrErr)
       return ToIFieldOrErr.takeError();
 
-    auto MemberLocOrErr = Import_New(From->getMemberLocation());
+    auto MemberLocOrErr = Import(From->getMemberLocation());
     if (!MemberLocOrErr)
       return MemberLocOrErr.takeError();
 
     return new (ToContext) CXXCtorInitializer(
         ToContext, cast_or_null<IndirectFieldDecl>(*ToIFieldOrErr),
         *MemberLocOrErr, *LParenLocOrErr, *ToExprOrErr, *RParenLocOrErr);
   } else if (From->isDelegatingInitializer()) {
-    auto ToTInfoOrErr = Import_New(From->getTypeSourceInfo());
+    auto ToTInfoOrErr = Import(From->getTypeSourceInfo());
     if (!ToTInfoOrErr)
       return ToTInfoOrErr.takeError();
 
     return new (ToContext)
         CXXCtorInitializer(ToContext, *ToTInfoOrErr, *LParenLocOrErr,
                            *ToExprOrErr, *RParenLocOrErr);
   } else {
     // FIXME: assert?
     return make_error<ImportError>();
   }
 }
-CXXCtorInitializer *ASTImporter::Import(CXXCtorInitializer *From) {
-  llvm::Expected<CXXCtorInitializer *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
 Expected<CXXBaseSpecifier *>
-ASTImporter::Import_New(const CXXBaseSpecifier *BaseSpec) {
+ASTImporter::Import(const CXXBaseSpecifier *BaseSpec) {
   auto Pos = ImportedCXXBaseSpecifiers.find(BaseSpec);
   if (Pos != ImportedCXXBaseSpecifiers.end())
     return Pos->second;
 
-  Expected<SourceRange> ToSourceRange = Import_New(BaseSpec->getSourceRange());
+  Expected<SourceRange> ToSourceRange = Import(BaseSpec->getSourceRange());
   if (!ToSourceRange)
     return ToSourceRange.takeError();
-  Expected<TypeSourceInfo *> ToTSI = Import_New(BaseSpec->getTypeSourceInfo());
+  Expected<TypeSourceInfo *> ToTSI = Import(BaseSpec->getTypeSourceInfo());
   if (!ToTSI)
     return ToTSI.takeError();
-  ExpectedSLoc ToEllipsisLoc = Import_New(BaseSpec->getEllipsisLoc());
+  ExpectedSLoc ToEllipsisLoc = Import(BaseSpec->getEllipsisLoc());
   if (!ToEllipsisLoc)
     return ToEllipsisLoc.takeError();
   CXXBaseSpecifier *Imported = new (ToContext) CXXBaseSpecifier(
       *ToSourceRange, BaseSpec->isVirtual(), BaseSpec->isBaseOfClass(),
       BaseSpec->getAccessSpecifierAsWritten(), *ToTSI, *ToEllipsisLoc);
   ImportedCXXBaseSpecifiers[BaseSpec] = Imported;
   return Imported;
 }
-CXXBaseSpecifier *ASTImporter::Import(const CXXBaseSpecifier *From) {
-  llvm::Expected<CXXBaseSpecifier *> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return nullptr;
-}
 
-Error ASTImporter::ImportDefinition_New(Decl *From) {
-  Decl *To = Import(From);
-  if (!To)
-    return llvm::make_error<ImportError>();
+Error ASTImporter::ImportDefinition(Decl *From) {
+  ExpectedDecl ToOrErr = Import(From);
+  if (!ToOrErr)
+    return ToOrErr.takeError();
+  Decl *To = *ToOrErr;
 
   auto *FromDC = cast<DeclContext>(From);
   ASTNodeImporter Importer(*this);
 
   if (auto *ToRecord = dyn_cast<RecordDecl>(To)) {
     if (!ToRecord->getDefinition()) {
       return Importer.ImportDefinition(
           cast<RecordDecl>(FromDC), ToRecord,
if (!ToProto->getDefinition()) {
           cast<ObjCProtocolDecl>(FromDC), ToProto,
           ASTNodeImporter::IDK_Everything);
     }
   }
 
   return Importer.ImportDeclContext(FromDC, true);
 }
 
-void ASTImporter::ImportDefinition(Decl *From) {
-  Error Err = ImportDefinition_New(From);
-  llvm::consumeError(std::move(Err));
-}
-
-Expected<DeclarationName> ASTImporter::Import_New(DeclarationName FromName) {
+Expected<DeclarationName> ASTImporter::Import(DeclarationName FromName) {
   if (!FromName)
     return DeclarationName{};
 
   switch (FromName.getNameKind()) {
   case DeclarationName::Identifier:
     return DeclarationName(Import(FromName.getAsIdentifierInfo()));
 
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
-    if (auto ToSelOrErr = Import_New(FromName.getObjCSelector()))
+    if (auto ToSelOrErr = Import(FromName.getObjCSelector()))
       return DeclarationName(*ToSelOrErr);
     else
       return ToSelOrErr.takeError();
 
   case DeclarationName::CXXConstructorName: {
-    if (auto ToTyOrErr = Import_New(FromName.getCXXNameType()))
+    if (auto ToTyOrErr = Import(FromName.getCXXNameType()))
       return ToContext.DeclarationNames.getCXXConstructorName(
           ToContext.getCanonicalType(*ToTyOrErr));
     else
       return ToTyOrErr.takeError();
   }
 
   case DeclarationName::CXXDestructorName: {
-    if (auto ToTyOrErr = Import_New(FromName.getCXXNameType()))
+    if (auto ToTyOrErr = Import(FromName.getCXXNameType()))
       return ToContext.DeclarationNames.getCXXDestructorName(
           ToContext.getCanonicalType(*ToTyOrErr));
     else
       return ToTyOrErr.takeError();
   }
 
   case DeclarationName::CXXDeductionGuideName: {
-    if (auto ToTemplateOrErr =
-            Import_New(FromName.getCXXDeductionGuideTemplate()))
+    if (auto ToTemplateOrErr = Import(FromName.getCXXDeductionGuideTemplate()))
       return ToContext.DeclarationNames.getCXXDeductionGuideName(
           cast<TemplateDecl>(*ToTemplateOrErr));
     else
       return ToTemplateOrErr.takeError();
   }
 
   case DeclarationName::CXXConversionFunctionName: {
-    if (auto ToTyOrErr = Import_New(FromName.getCXXNameType()))
+    if (auto ToTyOrErr = Import(FromName.getCXXNameType()))
       return ToContext.DeclarationNames.getCXXConversionFunctionName(
           ToContext.getCanonicalType(*ToTyOrErr));
     else
       return ToTyOrErr.takeError();
   }
 
   case DeclarationName::CXXOperatorName:
     return ToContext.DeclarationNames.getCXXOperatorName(
                                           FromName.getCXXOverloadedOperator());
 
   case DeclarationName::CXXLiteralOperatorName:
     return ToContext.DeclarationNames.getCXXLiteralOperatorName(
         Import(FromName.getCXXLiteralIdentifier()));
 
   case DeclarationName::CXXUsingDirective:
     // FIXME: STATICS!
     return DeclarationName::getUsingDirectiveName();
   }
 
   llvm_unreachable("Invalid DeclarationName Kind!");
 }
-DeclarationName ASTImporter::Import(DeclarationName From) {
-  llvm::Expected<DeclarationName> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
 IdentifierInfo *ASTImporter::Import(const IdentifierInfo *FromId) {
   if (!FromId)
     return nullptr;
 
   IdentifierInfo *ToId = &ToContext.Idents.get(FromId->getName());
 
   if (!ToId->getBuiltinID() && FromId->getBuiltinID())
     ToId->setBuiltinID(FromId->getBuiltinID());
 
   return ToId;
 }
 
-Expected<Selector> ASTImporter::Import_New(Selector FromSel) {
+Expected<Selector> ASTImporter::Import(Selector FromSel) {
   if (FromSel.isNull())
     return Selector{};
 
   SmallVector<IdentifierInfo *, 4> Idents;
   Idents.push_back(Import(FromSel.getIdentifierInfoForSlot(0)));
   for (unsigned I = 1, N = FromSel.getNumArgs(); I < N; ++I)
     Idents.push_back(Import(FromSel.getIdentifierInfoForSlot(I)));
   return ToContext.Selectors.getSelector(FromSel.getNumArgs(), Idents.data());
 }
 
 DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name,
                                                 DeclContext *DC,
                                                 unsigned IDNS,
                                                 NamedDecl **Decls,
                                                 unsigned NumDecls) {
   return Name;
 }
-Selector ASTImporter::Import(Selector From) {
-  llvm::Expected<Selector> To = Import_New(From);
-  if (To)
-    return *To;
-  else
-    llvm::consumeError(To.takeError());
-  return {};
-}
 
 DiagnosticBuilder ASTImporter::ToDiag(SourceLocation Loc, unsigned DiagID) {
   if (LastDiagFromFrom)
     ToContext.getDiagnostics().notePriorDiagnosticFrom(
       FromContext.getDiagnostics());
   LastDiagFromFrom = false;
   return ToContext.getDiagnostics().Report(Loc, DiagID);
 }
Decl *ASTImporter::MapImported(Decl *From, Decl *To) {
   return To;
 }
 
 bool ASTImporter::IsStructurallyEquivalent(QualType From, QualType To,
                                            bool Complain) {
   llvm::DenseMap<const Type *, const Type *>::iterator Pos =
       ImportedTypes.find(From.getTypePtr());
   if (Pos != ImportedTypes.end()) {
-    if (ExpectedType ToFromOrErr = Import_New(From)) {
+    if (ExpectedType ToFromOrErr = Import(From)) {
       if (ToContext.hasSameType(*ToFromOrErr, To))
         return true;
     } else {
       llvm::consumeError(ToFromOrErr.takeError());
     }
   }
 
   StructuralEquivalenceContext Ctx(FromContext, ToContext, NonEquivalentDecls,
                                    getStructuralEquivalenceKind(*this), false,
                                    Complain);
   return Ctx.IsEquivalent(From, To);
 }

cfe/trunk/lib/AST/ExternalASTMerger.cpp

LookupSameContext(Source<TranslationUnitDecl *> SourceTU, const DeclContext *DC,
   Source<const DeclContext *> SourceParentDC =
       LookupSameContext(SourceTU, DC->getParent(), ReverseImporter);
   if (!SourceParentDC) {
     // If we couldn't find the parent DC in this TranslationUnit, give up.
     return nullptr;
   }
   auto *ND = cast<NamedDecl>(DC);
   DeclarationName Name = ND->getDeclName();
-  auto SourceNameOrErr = ReverseImporter.Import_New(Name);
+  auto SourceNameOrErr = ReverseImporter.Import(Name);
   if (!SourceNameOrErr) {
     llvm::consumeError(SourceNameOrErr.takeError());
     return nullptr;
   }
   Source<DeclarationName> SourceName = *SourceNameOrErr;
   DeclContext::lookup_result SearchResult =
       SourceParentDC.get()->lookup(SourceName.get());
   size_t SearchResultSize = SearchResult.size();
void ExternalASTMerger::CompleteType(TagDecl *Tag) {
   ForEachMatchingDC(Tag, [&](ASTImporter &Forward, ASTImporter &Reverse,
                              Source<const DeclContext *> SourceDC) -> bool {
     auto *SourceTag = const_cast<TagDecl *>(cast<TagDecl>(SourceDC.get()));
     if (SourceTag->hasExternalLexicalStorage())
       SourceTag->getASTContext().getExternalSource()->CompleteType(SourceTag);
     if (!SourceTag->getDefinition())
       return false;
     Forward.MapImported(SourceTag, Tag);
-    if (llvm::Error Err = Forward.ImportDefinition_New(SourceTag))
+    if (llvm::Error Err = Forward.ImportDefinition(SourceTag))
       llvm::consumeError(std::move(Err));
     Tag->setCompleteDefinition(SourceTag->isCompleteDefinition());
     return true;
   });
 }
 
 void ExternalASTMerger::CompleteType(ObjCInterfaceDecl *Interface) {
   assert(Interface->hasExternalLexicalStorage());
   ForEachMatchingDC(
       Interface, [&](ASTImporter &Forward, ASTImporter &Reverse,
                      Source<const DeclContext *> SourceDC) -> bool {
         auto *SourceInterface = const_cast<ObjCInterfaceDecl *>(
             cast<ObjCInterfaceDecl>(SourceDC.get()));
         if (SourceInterface->hasExternalLexicalStorage())
           SourceInterface->getASTContext().getExternalSource()->CompleteType(
               SourceInterface);
         if (!SourceInterface->getDefinition())
           return false;
         Forward.MapImported(SourceInterface, Interface);
-        if (llvm::Error Err = Forward.ImportDefinition_New(SourceInterface))
+        if (llvm::Error Err = Forward.ImportDefinition(SourceInterface))
           llvm::consumeError(std::move(Err));
         return true;
       });
 }
 
 bool ExternalASTMerger::CanComplete(DeclContext *Interface) {
   assert(Interface->hasExternalLexicalStorage() ||
          Interface->hasExternalVisibleStorage());
for (OriginMap::iterator OI = Origins.begin(), OE = Origins.end(); OI != OE; ) {
     else
       ++OI;
   }
 }
 
 template <typename DeclTy>
 static bool importSpecializations(DeclTy *D, ASTImporter *Importer) {
   for (auto *Spec : D->specializations()) {
-    auto ImportedSpecOrError = Importer->Import_New(Spec);
+    auto ImportedSpecOrError = Importer->Import(Spec);
     if (!ImportedSpecOrError) {
       llvm::consumeError(ImportedSpecOrError.takeError());
       return true;
     }
   }
   return false;
 }
 
bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC,
   auto FilterFoundDecl = [&Candidates](const Candidate &C) {
    if (!HasDeclOfSameType(Candidates, C))
      Candidates.push_back(C);
   };
 
   ForEachMatchingDC(DC,
                     [&](ASTImporter &Forward, ASTImporter &Reverse,
                         Source<const DeclContext *> SourceDC) -> bool {
-                      auto FromNameOrErr = Reverse.Import_New(Name);
+                      auto FromNameOrErr = Reverse.Import(Name);
                       if (!FromNameOrErr) {
                         llvm::consumeError(FromNameOrErr.takeError());
                         return false;
                       }
                       DeclContextLookupResult Result =
                           SourceDC.get()->lookup(*FromNameOrErr);
                       for (NamedDecl *FromD : Result) {
                         FilterFoundDecl(std::make_pair(FromD, &Forward));
                       }
                       return false;
                     });
 
   if (Candidates.empty())
     return false;
 
   Decls.reserve(Candidates.size());
   for (const Candidate &C : Candidates) {
     Decl *LookupRes = C.first.get();
     ASTImporter *Importer = C.second;
-    auto NDOrErr = Importer->Import_New(LookupRes);
+    auto NDOrErr = Importer->Import(LookupRes);
     assert(NDOrErr);
     (void)static_cast<bool>(NDOrErr);
     NamedDecl *ND = cast_or_null<NamedDecl>(*NDOrErr);
     assert(ND);
     // If we don't import specialization, they are not available via lookup
     // because the lookup result is imported TemplateDecl and it does not
     // reference its specializations until they are imported explicitly.
     bool IsSpecImportFailed =
         importSpecializationsIfNeeded(LookupRes, Importer);
     assert(!IsSpecImportFailed);
     (void)IsSpecImportFailed;
     Decls.push_back(ND);
   }
   SetExternalVisibleDeclsForName(DC, Name, Decls);
   return true;
 }
 
 void ExternalASTMerger::FindExternalLexicalDecls(
     const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
     SmallVectorImpl<Decl *> &Result) {
   ForEachMatchingDC(DC, [&](ASTImporter &Forward, ASTImporter &Reverse,
                             Source<const DeclContext *> SourceDC) -> bool {
     for (const Decl *SourceDecl : SourceDC.get()->decls()) {
       if (IsKindWeWant(SourceDecl->getKind())) {
-        auto ImportedDeclOrErr = Forward.Import_New(SourceDecl);
+        auto ImportedDeclOrErr = Forward.Import(SourceDecl);
         if (ImportedDeclOrErr)
           assert(!(*ImportedDeclOrErr) ||
                  IsSameDC((*ImportedDeclOrErr)->getDeclContext(), DC));
         else
           llvm::consumeError(ImportedDeclOrErr.takeError());
       }
     }
     return false;
   });
 }
 

cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

 }
 
 template <typename T>
 llvm::Expected<const T *>
 CrossTranslationUnitContext::importDefinitionImpl(const T *D) {
   assert(hasBodyOrInit(D) && "Decls to be imported should have body or init.");
 
   ASTImporter &Importer = getOrCreateASTImporter(D->getASTContext());
-  auto ToDeclOrError = Importer.Import_New(D);
+  auto ToDeclOrError = Importer.Import(D);
   if (!ToDeclOrError) {
     handleAllErrors(ToDeclOrError.takeError(),
                     [&](const ImportError &IE) {
                       switch (IE.Error) {
                       case ImportError::NameConflict:
                         // FIXME: Add statistic.
                          break;
                       case ImportError::UnsupportedConstruct:

cfe/trunk/lib/Frontend/ASTMerge.cpp

for (unsigned I = 0, N = ASTFiles.size(); I != N; ++I) {
     TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
     for (auto *D : TU->decls()) {
       // Don't re-import __va_list_tag, __builtin_va_list.
       if (const auto *ND = dyn_cast<NamedDecl>(D))
         if (IdentifierInfo *II = ND->getIdentifier())
           if (II->isStr("__va_list_tag") || II->isStr("__builtin_va_list"))
             continue;
 
-      llvm::Expected<Decl *> ToDOrError = Importer.Import_New(D);
+      llvm::Expected<Decl *> ToDOrError = Importer.Import(D);
 
       if (ToDOrError) {
         DeclGroupRef DGR(*ToDOrError);
         CI.getASTConsumer().HandleTopLevelDecl(DGR);
       } else {
         llvm::consumeError(ToDOrError.takeError());
       }
     }

cfe/trunk/unittests/AST/ASTImporterFixtures.cpp

Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
                            &LookupTable));
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
 
 Decl *ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
                                       ASTUnit *ToAST, Decl *FromDecl) {
   lazyInitImporter(LookupTable, ToAST);
-  if (auto ImportedOrErr = Importer->Import_New(FromDecl))
+  if (auto ImportedOrErr = Importer->Import(FromDecl))
     return *ImportedOrErr;
   else {
     llvm::consumeError(ImportedOrErr.takeError());
     return nullptr;
   }
 }
 
 QualType ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
                                          ASTUnit *ToAST, QualType FromType) {
   lazyInitImporter(LookupTable, ToAST);
-  if (auto ImportedOrErr = Importer->Import_New(FromType))
+  if (auto ImportedOrErr = Importer->Import(FromType))
     return *ImportedOrErr;
   else {
     llvm::consumeError(ImportedOrErr.takeError());
     return QualType{};
   }
 }
 
 void ASTImporterTestBase::lazyInitLookupTable(TranslationUnitDecl *ToTU) {

cfe/trunk/unittests/AST/ASTImporterTest.cpp

llvm::Expected<NodeType> importNode(ASTUnit *From, ASTUnit *To,
 
     // Add 'From' file to virtual file system so importer can 'find' it
     // while importing SourceLocations. It is safe to add same file multiple
     // times - it just isn't replaced.
     StringRef FromFileName = From->getMainFileName();
     createVirtualFileIfNeeded(To, FromFileName,
                               From->getBufferForFile(FromFileName));
 
-    auto Imported = Importer.Import_New(Node);
+    auto Imported = Importer.Import(Node);
 
     if (Imported) {
       // This should dump source locations and assert if some source locations
       // were not imported.
       SmallString<1024> ImportChecker;
       llvm::raw_svector_ostream ToNothing(ImportChecker);
       ToCtx.getTranslationUnitDecl()->print(ToNothing);
 

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

clang::QualType ClangASTSource::CopyTypeWithMerger(
     clang::ASTContext &from_context,
     clang::ExternalASTMerger &merger,
     clang::QualType type) {
   if (!merger.HasImporterForOrigin(from_context)) {
     lldbassert(0 && "Couldn't find the importer for a source context!");
     return QualType();
   }
 
-  return merger.ImporterForOrigin(from_context).Import(type);
+  if (llvm::Expected<QualType> type_or_error =
+          merger.ImporterForOrigin(from_context).Import(type)) {
+    return *type_or_error;
+  } else {
+    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+    LLDB_LOG_ERROR(log, type_or_error.takeError(), "Couldn't import type: {0}");
+    return QualType();
+  }
 }
 
 clang::Decl *ClangASTSource::CopyDecl(Decl *src_decl) {
   clang::ASTContext &from_context = src_decl->getASTContext();
   if (m_ast_importer_sp) {
     return m_ast_importer_sp->CopyDecl(m_ast_context, &from_context, src_decl);
   } else if (m_merger_up) {
     if (!m_merger_up->HasImporterForOrigin(from_context)) {
       lldbassert(0 && "Couldn't find the importer for a source context!");
       return nullptr;
     }
 
-    return m_merger_up->ImporterForOrigin(from_context).Import(src_decl);
+    if (llvm::Expected<Decl *> decl_or_error =
+            m_merger_up->ImporterForOrigin(from_context).Import(src_decl)) {
+      return *decl_or_error;
+    } else {
+      Log *log =
+          lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+      LLDB_LOG_ERROR(log, decl_or_error.takeError(),
+                     "Couldn't import decl: {0}");
+      return nullptr;
+    }
   } else {
     lldbassert(0 && "No mechanism for copying a decl!");
     return nullptr;
   }
 }
 
 bool ClangASTSource::ResolveDeclOrigin(const clang::Decl *decl,
                                        clang::Decl **original_decl,

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

static clang::QualType ExportAllDeclaredTypes(
     clang::ASTContext &source, clang::FileManager &source_file_manager,
     const clang::ExternalASTMerger::OriginMap &source_origin_map,
     clang::FileID file, clang::QualType root) {
   clang::ExternalASTMerger::ImporterSource importer_source =
       { source, source_file_manager, source_origin_map };
   merger.AddSources(importer_source);
   clang::ASTImporter &exporter = merger.ImporterForOrigin(source);
   CompleteAllDeclContexts(exporter, file, root);
-  clang::QualType ret = exporter.Import(root);
+  llvm::Expected<clang::QualType> ret_or_error = exporter.Import(root);
   merger.RemoveSources(importer_source);
-  return ret;
+  if (ret_or_error) {
+    return *ret_or_error;
+  } else {
+    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+    LLDB_LOG_ERROR(log, ret_or_error.takeError(), "Couldn't import type: {0}");
+    return clang::QualType();
+  }
 }
 
 TypeFromUser ClangExpressionDeclMap::DeportType(ClangASTContext &target,
                                                 ClangASTContext &source,
                                                 TypeFromParser parser_type) {
   assert (&target == m_target->GetScratchClangASTContext());
   assert ((TypeSystem*)&source == parser_type.GetTypeSystem());
   assert (source.getASTContext() == m_ast_context);

lldb/trunk/source/Symbol/ClangASTContext.cpp

 }
 
 clang::Decl *ClangASTContext::CopyDecl(ASTContext *dst_ast, ASTContext *src_ast,
                                        clang::Decl *source_decl) {
   FileSystemOptions file_system_options;
   FileManager file_manager(file_system_options);
   ASTImporter importer(*dst_ast, file_manager, *src_ast, file_manager, false);
 
-  return importer.Import(source_decl);
+  if (llvm::Expected<clang::Decl *> ret_or_error =
+          importer.Import(source_decl)) {
+    return *ret_or_error;
+  } else {
+    Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+    LLDB_LOG_ERROR(log, ret_or_error.takeError(), "Couldn't import decl: {0}");
+    return nullptr;
+  }
 }
 
 bool ClangASTContext::AreTypesSame(CompilerType type1, CompilerType type2,
                                    bool ignore_qualifiers) {
   ClangASTContext *ast =
       llvm::dyn_cast_or_null<ClangASTContext>(type1.GetTypeSystem());
   if (!ast || ast != type2.GetTypeSystem())
     return false;

lldb/trunk/source/Symbol/ClangASTImporter.cpp

 
 clang::QualType ClangASTImporter::CopyType(clang::ASTContext *dst_ast,
                                            clang::ASTContext *src_ast,
                                            clang::QualType type) {
   ImporterDelegateSP delegate_sp(GetDelegate(dst_ast, src_ast));
 
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, dst_ast);
 
-  if (delegate_sp)
-    return delegate_sp->Import(type);
+  if (!delegate_sp)
+    return QualType();
 
+  llvm::Expected<QualType> ret_or_error = delegate_sp->Import(type);
+  if (!ret_or_error) {
+    Log *log =
+      lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+    LLDB_LOG_ERROR(log, ret_or_error.takeError(),
+        "Couldn't import type: {0}");
     return QualType();
   }
+  return *ret_or_error;
+}
 
 lldb::opaque_compiler_type_t
 ClangASTImporter::CopyType(clang::ASTContext *dst_ast,
                            clang::ASTContext *src_ast,
                            lldb::opaque_compiler_type_t type) {
   return CopyType(dst_ast, src_ast, QualType::getFromOpaquePtr(type))
       .getAsOpaquePtr();
 }
clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext *dst_ast,
                                         clang::ASTContext *src_ast,
                                         clang::Decl *decl) {
   ImporterDelegateSP delegate_sp;
 
   delegate_sp = GetDelegate(dst_ast, src_ast);
 
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, dst_ast);
 
-  if (delegate_sp) {
-    clang::Decl *result = delegate_sp->Import(decl);
+  if (!delegate_sp)
+    return nullptr;
 
-    if (!result) {
-      Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
-      if (log) {
-        lldb::user_id_t user_id = LLDB_INVALID_UID;
-        ClangASTMetadata *metadata = GetDeclMetadata(decl);
-        if (metadata)
-          user_id = metadata->GetUserID();
+  llvm::Expected<clang::Decl *> result = delegate_sp->Import(decl);
+    LLDB_LOG_ERROR(log, result.takeError(), "Couldn't import decl: {0}");
 
-        if (NamedDecl *named_decl = dyn_cast<NamedDecl>(decl))
-          log->Printf("  [ClangASTImporter] WARNING: Failed to import a %s "
-                      "'%s', metadata 0x%" PRIx64,
-                      decl->getDeclKindName(),
-                      named_decl->getNameAsString().c_str(), user_id);
-        else
-          log->Printf("  [ClangASTImporter] WARNING: Failed to import a %s, "
-                      "metadata 0x%" PRIx64,
-                      decl->getDeclKindName(), user_id);
-      }
-    }
+    return nullptr;
 
-    return result;
-  }
-
-  return nullptr;
+  return *result;
 }
 
 class DeclContextOverride {
 private:
   struct Backup {
     clang::DeclContext *decl_context;
     clang::DeclContext *lexical_decl_context;
   };
bool ClangASTImporter::CompleteObjCInterfaceDecl(
 
   return true;
 }
 
 bool ClangASTImporter::CompleteAndFetchChildren(clang::QualType type) {
   if (!RequireCompleteType(type))
     return false;
 
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
   if (const TagType *tag_type = type->getAs<TagType>()) {
     TagDecl *tag_decl = tag_type->getDecl();
 
     DeclOrigin decl_origin = GetDeclOrigin(tag_decl);
 
     if (!decl_origin.Valid())
       return false;
 
     ImporterDelegateSP delegate_sp(
         GetDelegate(&tag_decl->getASTContext(), decl_origin.ctx));
 
     ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp,
                                                   &tag_decl->getASTContext());
 
     TagDecl *origin_tag_decl = llvm::dyn_cast<TagDecl>(decl_origin.decl);
 
     for (Decl *origin_child_decl : origin_tag_decl->decls()) {
+      llvm::Expected<Decl *> imported_or_err =
           delegate_sp->Import(origin_child_decl);
+      if (!imported_or_err) {
+        LLDB_LOG_ERROR(log, imported_or_err.takeError(),
+                       "Couldn't import decl: {0}");
+        return false;
+      }
     }
 
     if (RecordDecl *record_decl = dyn_cast<RecordDecl>(origin_tag_decl)) {
       record_decl->setHasLoadedFieldsFromExternalStorage(true);
     }
 
     return true;
   }
 
   if (const ObjCObjectType *objc_object_type = type->getAs<ObjCObjectType>()) {
     if (ObjCInterfaceDecl *objc_interface_decl =
             objc_object_type->getInterface()) {
       DeclOrigin decl_origin = GetDeclOrigin(objc_interface_decl);
 
       if (!decl_origin.Valid())
         return false;
 
       ImporterDelegateSP delegate_sp(
           GetDelegate(&objc_interface_decl->getASTContext(), decl_origin.ctx));
 
       ObjCInterfaceDecl *origin_interface_decl =
           llvm::dyn_cast<ObjCInterfaceDecl>(decl_origin.decl);
 
       for (Decl *origin_child_decl : origin_interface_decl->decls()) {
+        llvm::Expected<Decl *> imported_or_err =
             delegate_sp->Import(origin_child_decl);
+        if (!imported_or_err) {
+          LLDB_LOG_ERROR(log, imported_or_err.takeError(),
+                         "Couldn't import decl: {0}");
+          return false;
+        }
       }
 
       return true;
     } else {
       return false;
     }
   }
 
if (to_objc_interface)
       to_objc_interface->startDefinition();
 
   CXXRecordDecl *to_cxx_record = dyn_cast<CXXRecordDecl>(to);
 
   if (to_cxx_record)
       to_cxx_record->startDefinition();
   */
 
-  ImportDefinition(from);
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
+  if (llvm::Error err = ImportDefinition(from)) {
+    LLDB_LOG_ERROR(log, std::move(err),
+                   "[ClangASTImporter] Error during importing definition: {0}");
+    return;
+  }
 
   if (clang::TagDecl *to_tag = dyn_cast<clang::TagDecl>(to)) {
     if (clang::TagDecl *from_tag = dyn_cast<clang::TagDecl>(from)) {
       to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
     }
   }
 
   // If we're dealing with an Objective-C class, ensure that the inheritance
do {
       if (!from_objc_interface)
         break;
 
       ObjCInterfaceDecl *from_superclass = from_objc_interface->getSuperClass();
 
       if (!from_superclass)
         break;
 
-      Decl *imported_from_superclass_decl = Import(from_superclass);
+      llvm::Expected<Decl *> imported_from_superclass_decl =
+          Import(from_superclass);
 
-      if (!imported_from_superclass_decl)
+      if (!imported_from_superclass_decl) {
+        LLDB_LOG_ERROR(log, imported_from_superclass_decl.takeError(),
+                       "Couldn't import decl: {0}");
         break;
+      }
 
       ObjCInterfaceDecl *imported_from_superclass =
-          dyn_cast<ObjCInterfaceDecl>(imported_from_superclass_decl);
+          dyn_cast<ObjCInterfaceDecl>(*imported_from_superclass_decl);
 
       if (!imported_from_superclass)
         break;
 
       if (!to_objc_interface->hasDefinition())
         to_objc_interface->startDefinition();
 
       to_objc_interface->setSuperClass(m_source_ctx->getTrivialTypeSourceInfo(

lldb/trunk/source/Symbol/CxxModuleHandler.cpp

 //===-- CxxModuleHandler.cpp ------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Symbol/CxxModuleHandler.h"
 
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Utility/Log.h"
 #include "clang/Sema/Lookup.h"
 #include "llvm/Support/Error.h"
 
 using namespace lldb_private;
 using namespace clang;
 
 CxxModuleHandler::CxxModuleHandler(ASTImporter &importer, ASTContext *target)
     : m_importer(&importer),
if ((new_class_template = dyn_cast<ClassTemplateDecl>(LD)))
       break;
   }
   if (!new_class_template)
     return {};
 
   // Import the foreign template arguments.
   llvm::SmallVector<TemplateArgument, 4> imported_args;
 
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
   // If this logic is changed, also update templateArgsAreSupported.
   for (const TemplateArgument &arg : foreign_args.asArray()) {
     switch (arg.getKind()) {
     case TemplateArgument::Type: {
-      llvm::Expected<QualType> type = m_importer->Import_New(arg.getAsType());
+      llvm::Expected<QualType> type = m_importer->Import(arg.getAsType());
       if (!type) {
-        llvm::consumeError(type.takeError());
+        LLDB_LOG_ERROR(log, type.takeError(), "Couldn't import type: {0}");
         return {};
       }
       imported_args.push_back(TemplateArgument(*type));
       break;
     }
     case TemplateArgument::Integral: {
       llvm::APSInt integral = arg.getAsIntegral();
       llvm::Expected<QualType> type =
-          m_importer->Import_New(arg.getIntegralType());
+          m_importer->Import(arg.getIntegralType());
       if (!type) {
-        llvm::consumeError(type.takeError());
+        LLDB_LOG_ERROR(log, type.takeError(), "Couldn't import type: {0}");
         return {};
       }
       imported_args.push_back(
           TemplateArgument(d->getASTContext(), integral, *type));
       break;
     }
     default:
       assert(false && "templateArgsAreSupported not updated?");