[ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

[balazske]

Rebase to master.

[balazske]

• Changed every use of ASTImporter::Import to Import_New

[balazske]

Removing previous wrong update.

[martong]

Ping. This patch is the next in the series of patches to finish the proper error handling (i.e. using Error and Expected<T>). Please see the "Stack" column above.

[martong]

This patch is the next in the series of patches to finish the proper error handling (i.e. using Error and Expected<T>). Please see the "Stack" column above.

@balazske Or is it superseded by this one? https://reviews.llvm.org/D53818

[balazske]

Rebase (old patch applied without changes).

[balazske]

This is the next part of the changes for error handling. D53818 was for ASTImporter functions only, this is for ASTNodeImporter (the Import_New functions are not used at many places directly because mostly the few import templates are used). The next part is D55049 which switches to Import_New in every other place (in clang source code only). Next part would be to use Import_New in lldb, then rename all Import_New to Import (and remove old Import).

[martong]

@shafik, @a_sidorin Ping. Could you please take a look?
Guys, this and its child patch are very important patches, because without it the error handling of ASTImporter is not completed. This means we may encounter a nullptr DeclContext in the middle of the import process, etc...

[shafik]

lib/AST/ASTImporter.cpp

3418

Why is this section of code removed?

[a_sidorin]

Hi Balasz,

Sorry, I missed the review accidentally. Thank you for the patch!

lib/AST/ASTImporter.cpp

3418

I guess the reason is that this import is already done inside GetImportedOrCreateDecl().

[balazske]

lib/AST/ASTImporter.cpp

3418

You are right: This is done in InitializeImportedDecl that is called from GetImportedOrCreateDecl.

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 ToI = Importer.Import(From);
-      if (!ToI && From)
-        return make_error<ImportError>();
-      To = cast_or_null<ImportT>(ToI);
-      return Error::success();
-      // FIXME: This should be the final code.
-      //auto ToOrErr = Importer.Import(From);
-      //if (ToOrErr) {
-      //  To = cast_or_null<ImportT>(*ToOrErr);
-      //}
-      //return ToOrErr.takeError();
+      auto ToOrErr = Importer.Import_New(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 *To = Importer.Import(From);
-      if (!To && From)
-        return make_error<ImportError>();
-      return cast_or_null<T>(To);
-      // FIXME: This should be the final code.
-      //auto ToOrErr = Importer.Import(From);
-      //if (!ToOrErr)
-      //  return ToOrErr.takeError();
-      //return cast_or_null<T>(*ToOrErr);
+      auto ToOrErr = Importer.Import_New(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) {
-      T To = Importer.Import(From);
-      T DefaultT;
-      if (To == DefaultT && !(From == DefaultT))
-        return make_error<ImportError>();
-      return To;
-      // FIXME: This should be the final code.
-      //return Importer.Import(From);
+      return Importer.Import_New(From);
     }
 
     template <class T>
     Expected<std::tuple<T>>
     importSeq(const T &From) {
       Expected<T> ToOrErr = import(From);
       if (!ToOrErr)
         return ToOrErr.takeError();
GetImportedOrCreateSpecialDecl(ToDeclT *&ToD, CreateFunT CreateFun,
       Importer.AddToLookupTable(ToD);
       InitializeImportedDecl(FromD, ToD);
       return false; // A new Decl is created.
     }
 
     void InitializeImportedDecl(Decl *FromD, Decl *ToD) {
       ToD->IdentifierNamespace = FromD->IdentifierNamespace;
       if (FromD->hasAttrs())
-        for (const Attr *FromAttr : FromD->getAttrs())
-          ToD->addAttr(Importer.Import(FromAttr));
+        for (const Attr *FromAttr : FromD->getAttrs()) {
+          // FIXME: Return of the error here is not possible until store of
+          // import errors is implemented.
+          auto ToAttrOrErr = import(FromAttr);
+          if (ToAttrOrErr)
+            ToD->addAttr(*ToAttrOrErr);
+          else
+            llvm::consumeError(ToAttrOrErr.takeError());
+        }
       if (FromD->isUsed())
         ToD->setIsUsed();
       if (FromD->isImplicit())
         ToD->setImplicit();
     }
 
   public:
     explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
public:
     }
 
     void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);
 
     Expected<FunctionDecl *> FindFunctionTemplateSpecialization(
         FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected<TemplateName> ASTNodeImporter::import(const TemplateName &From) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-    return make_error<ImportError>();
-  return To;
-}
-
 template <typename InContainerTy>
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
     SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
     const InContainerTy &Container, TemplateArgumentListInfo &Result) {
   auto ToLAngleLocOrErr = import(FromLAngleLoc);
   if (!ToLAngleLocOrErr)
     return ToLAngleLocOrErr.takeError();
   auto ToRAngleLocOrErr = import(FromRAngleLoc);
for (CXXMethodDecl *FromM : From->methods())
     }
 
   return Error::success();
 }
 
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
                                        ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-    Decl *ToTypedef = Importer.Import(FromTypedef);
-    if (!ToTypedef)
-      return make_error<ImportError>();
-    To->setTypedefNameForAnonDecl(cast<TypedefNameDecl>(ToTypedef));
-    // FIXME: This should be the final code.
-    //if (Expected<Decl *> ToTypedefOrErr = Importer.Import(FromTypedef))
-    //  To->setTypedefNameForAnonDecl(cast<TypedefNameDecl>(*ToTypedefOrErr));
-    //else
-    //  return ToTypedefOrErr.takeError();
+    if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+      To->setTypedefNameForAnonDecl(cast<TypedefNameDecl>(*ToTypedefOrErr));
+    else
+      return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
 
 Error ASTNodeImporter::ImportDefinition(
     RecordDecl *From, RecordDecl *To, ImportDefinitionKind Kind) {
   if (To->getDefinition() || To->isBeingDefined()) {
     if (Kind == IDK_Everything)
ExpectedDecl ASTNodeImporter::VisitIndirectFieldDecl(IndirectFieldDecl *D) {
 
   llvm::MutableArrayRef<NamedDecl *> CH = {NamedChain, D->getChainingSize()};
   IndirectFieldDecl *ToIndirectField;
   if (GetImportedOrCreateDecl(ToIndirectField, D, Importer.getToContext(), DC,
                               Loc, Name.getAsIdentifierInfo(), *TypeOrErr, CH))
     // FIXME here we leak `NamedChain` which is allocated before
     return ToIndirectField;
 
-  for (const auto *Attr : D->attrs())
-    ToIndirectField->addAttr(Importer.Import(Attr));
-
   ToIndirectField->setAccess(D->getAccess());
   ToIndirectField->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToIndirectField);
   return ToIndirectField;
 }
 
 ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
   // Import the major distinguishing characteristics of a declaration.