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

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356874: [ASTImporter] Changed use of Import to Import_New in 
ASTNodeImporter. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -148,32 +148,20 @@
 // Use this to import pointers of specific type.
 template 
 LLVM_NODISCARD Error importInto(ImportT *, ImportT *From) {
-  auto ToI = Importer.Import(From);
-  if (!ToI && From)
-return make_error();
-  To = cast_or_null(ToI);
-  return Error::success();
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (ToOrErr) {
-  //  To = cast_or_null(*ToOrErr);
-  //}
-  //return ToOrErr.takeError();
+  auto ToOrErr = Importer.Import_New(From);
+  if (ToOrErr)
+To = cast_or_null(*ToOrErr);
+  return ToOrErr.takeError();
 }
 
 // Call the import function of ASTImporter for a baseclass of type `T` and
 // cast the return value to `T`.
 template 
 Expected import(T *From) {
-  auto *To = Importer.Import(From);
-  if (!To && From)
-return make_error();
-  return cast_or_null(To);
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (!ToOrErr)
-  //  return ToOrErr.takeError();
-  //return cast_or_null(*ToOrErr);
+  auto ToOrErr = Importer.Import_New(From);
+  if (!ToOrErr)
+return ToOrErr.takeError();
+  return cast_or_null(*ToOrErr);
 }
 
 template 
@@ -184,13 +172,7 @@
 // Call the import function of ASTImporter for type `T`.
 template 
 Expected import(const T ) {
-  T To = Importer.Import(From);
-  T DefaultT;
-  if (To == DefaultT && !(From == DefaultT))
-return make_error();
-  return To;
-  // FIXME: This should be the final code.
-  //return Importer.Import(From);
+  return Importer.Import_New(From);
 }
 
 template 
@@ -282,8 +264,15 @@
 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())
@@ -643,15 +632,6 @@
 FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected ASTNodeImporter::import(const TemplateName ) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-return make_error();
-  return To;
-}
-
 template 
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
 SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
@@ -1692,15 +1672,10 @@
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter ) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-Decl *ToTypedef = Importer.Import(FromTypedef);
-if (!ToTypedef)
-  return make_error();
-To->setTypedefNameForAnonDecl(cast(ToTypedef));
-// FIXME: This should be the final code.
-//if (Expected ToTypedefOrErr = Importer.Import(FromTypedef))
-//  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
-//else
-//  return ToTypedefOrErr.takeError();
+if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
+else
+  return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
@@ -3386,9 +3361,6 @@
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked an inline comment as done.
balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:3418
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Importer.Import(Attr));

a_sidorin wrote:
> shafik wrote:
> > Why is this section of code removed?
> I guess the reason is that this import is already done inside 
> `GetImportedOrCreateDecl()`.
You are right: This is done in `InitializeImportedDecl` that is called from 
`GetImportedOrCreateDecl`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Hi Balasz,

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




Comment at: lib/AST/ASTImporter.cpp:3418
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Importer.Import(Attr));

shafik wrote:
> Why is this section of code removed?
I guess the reason is that this import is already done inside 
`GetImportedOrCreateDecl()`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: lib/AST/ASTImporter.cpp:3418
 
-  for (const auto *Attr : D->attrs())
-ToIndirectField->addAttr(Importer.Import(Attr));

Why is this section of code removed?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a reviewer: a_sidorin.
martong added a subscriber: a_sidorin.
martong added a comment.

@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...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

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`).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 189824.
balazske added a comment.

Rebase (old patch applied without changes).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -148,32 +148,20 @@
 // Use this to import pointers of specific type.
 template 
 LLVM_NODISCARD Error importInto(ImportT *, ImportT *From) {
-  auto ToI = Importer.Import(From);
-  if (!ToI && From)
-return make_error();
-  To = cast_or_null(ToI);
-  return Error::success();
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (ToOrErr) {
-  //  To = cast_or_null(*ToOrErr);
-  //}
-  //return ToOrErr.takeError();
+  auto ToOrErr = Importer.Import_New(From);
+  if (ToOrErr)
+To = cast_or_null(*ToOrErr);
+  return ToOrErr.takeError();
 }
 
 // Call the import function of ASTImporter for a baseclass of type `T` and
 // cast the return value to `T`.
 template 
 Expected import(T *From) {
-  auto *To = Importer.Import(From);
-  if (!To && From)
-return make_error();
-  return cast_or_null(To);
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (!ToOrErr)
-  //  return ToOrErr.takeError();
-  //return cast_or_null(*ToOrErr);
+  auto ToOrErr = Importer.Import_New(From);
+  if (!ToOrErr)
+return ToOrErr.takeError();
+  return cast_or_null(*ToOrErr);
 }
 
 template 
@@ -184,13 +172,7 @@
 // Call the import function of ASTImporter for type `T`.
 template 
 Expected import(const T ) {
-  T To = Importer.Import(From);
-  T DefaultT;
-  if (To == DefaultT && !(From == DefaultT))
-return make_error();
-  return To;
-  // FIXME: This should be the final code.
-  //return Importer.Import(From);
+  return Importer.Import_New(From);
 }
 
 template 
@@ -282,8 +264,15 @@
 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())
@@ -635,15 +624,6 @@
 FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected ASTNodeImporter::import(const TemplateName ) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-return make_error();
-  return To;
-}
-
 template 
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
 SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
@@ -1684,15 +1664,10 @@
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter ) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-Decl *ToTypedef = Importer.Import(FromTypedef);
-if (!ToTypedef)
-  return make_error();
-To->setTypedefNameForAnonDecl(cast(ToTypedef));
-// FIXME: This should be the final code.
-//if (Expected ToTypedefOrErr = Importer.Import(FromTypedef))
-//  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
-//else
-//  return ToTypedefOrErr.takeError();
+if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
+else
+  return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
@@ -3415,9 +3390,6 @@
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.
Herald added a subscriber: rnkovacs.

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

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.
Herald added a reviewer: martong.
Herald added a project: clang.

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 175822.
balazske added a comment.

Removing previous wrong update.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -168,32 +168,20 @@
 // Use this to import pointers of specific type.
 template 
 LLVM_NODISCARD Error importInto(ImportT *, ImportT *From) {
-  auto ToI = Importer.Import(From);
-  if (!ToI && From)
-return make_error();
-  To = cast_or_null(ToI);
-  return Error::success();
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (ToOrErr) {
-  //  To = cast_or_null(*ToOrErr);
-  //}
-  //return ToOrErr.takeError();
+  auto ToOrErr = Importer.Import_New(From);
+  if (ToOrErr)
+To = cast_or_null(*ToOrErr);
+  return ToOrErr.takeError();
 }
 
 // Call the import function of ASTImporter for a baseclass of type `T` and
 // cast the return value to `T`.
 template 
 Expected import(T *From) {
-  auto *To = Importer.Import(From);
-  if (!To && From)
-return make_error();
-  return cast_or_null(To);
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (!ToOrErr)
-  //  return ToOrErr.takeError();
-  //return cast_or_null(*ToOrErr);
+  auto ToOrErr = Importer.Import_New(From);
+  if (!ToOrErr)
+return ToOrErr.takeError();
+  return cast_or_null(*ToOrErr);
 }
 
 template 
@@ -204,13 +192,7 @@
 // Call the import function of ASTImporter for type `T`.
 template 
 Expected import(const T ) {
-  T To = Importer.Import(From);
-  T DefaultT;
-  if (To == DefaultT && !(From == DefaultT))
-return make_error();
-  return To;
-  // FIXME: This should be the final code.
-  //return Importer.Import(From);
+  return Importer.Import_New(From);
 }
 
 template 
@@ -300,8 +282,15 @@
   Importer.MapImported(FromD, 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())
@@ -647,15 +636,6 @@
 FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected ASTNodeImporter::import(const TemplateName ) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-return make_error();
-  return To;
-}
-
 template 
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
 SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
@@ -1693,15 +1673,10 @@
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter ) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-Decl *ToTypedef = Importer.Import(FromTypedef);
-if (!ToTypedef)
-  return make_error();
-To->setTypedefNameForAnonDecl(cast(ToTypedef));
-// FIXME: This should be the final code.
-//if (Expected ToTypedefOrErr = Importer.Import(FromTypedef))
-//  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
-//else
-//  return ToTypedefOrErr.takeError();
+if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
+else
+  return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
@@ -3437,9 +3412,6 @@
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 175821.
balazske added a comment.

- Changed every use of ASTImporter::Import to Import_New


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757

Files:
  lib/AST/ASTImporter.cpp
  lib/AST/ExternalASTMerger.cpp
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/Frontend/ASTMerge.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -75,8 +75,8 @@
 class TestImportBase : public ParameterizedTestsFixture {
 
   template 
-  NodeType importNode(ASTUnit *From, ASTUnit *To, ASTImporter ,
-  NodeType Node) {
+  llvm::Expected importNode(ASTUnit *From, ASTUnit *To,
+  ASTImporter , NodeType Node) {
 ASTContext  = To->getASTContext();
 
 // Add 'From' file to virtual file system so importer can 'find' it
@@ -86,17 +86,19 @@
 createVirtualFileIfNeeded(To, FromFileName,
   From->getBufferForFile(FromFileName));
 
-auto Imported = Importer.Import(Node);
+auto Imported = Importer.Import_New(Node);
 
-// 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);
+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);
 
-// This traverses the AST to catch certain bugs like poorly or not
-// implemented subtrees.
-Imported->dump(ToNothing);
+  // This traverses the AST to catch certain bugs like poorly or not
+  // implemented subtrees.
+  (*Imported)->dump(ToNothing);
+}
 
 return Imported;
   }
@@ -137,11 +139,14 @@
 EXPECT_TRUE(Verifier.match(ToImport, WrapperMatcher));
 
 auto Imported = importNode(FromAST.get(), ToAST.get(), Importer, ToImport);
-if (!Imported)
-  return testing::AssertionFailure() << "Import failed, nullptr returned!";
-
+if (!Imported) {
+  llvm::Error Err = Imported.takeError();
+  return testing::AssertionFailure()
+ << "Import failed, error: \"" << Err << "\"!";
+  llvm::consumeError(std::move(Err));
+}
 
-return Verifier.match(Imported, WrapperMatcher);
+return Verifier.match(*Imported, WrapperMatcher);
   }
 
   template 
@@ -260,7 +265,9 @@
   EXPECT_TRUE(FoundDecl.size() == 1);
   const Decl *ToImport = selectFirst(DeclToImportID, FoundDecl);
   auto Imported = importNode(From, To, *ImporterRef, ToImport);
-  EXPECT_TRUE(Imported);
+  EXPECT_TRUE(static_cast(Imported));
+  if (!Imported)
+llvm::consumeError(Imported.takeError());
 }
 
 // Find the declaration and import it.
@@ -320,12 +327,22 @@
 
 Decl *import(ASTUnit *ToAST, Decl *FromDecl) {
   lazyInitImporter(ToAST);
-  return Importer->Import(FromDecl);
- }
+  if (auto ImportedOrErr = Importer->Import_New(FromDecl))
+return *ImportedOrErr;
+  else {
+llvm::consumeError(ImportedOrErr.takeError());
+return nullptr;
+  }
+}
 
 QualType import(ASTUnit *ToAST, QualType FromType) {
   lazyInitImporter(ToAST);
-  return Importer->Import(FromType);
+  if (auto ImportedOrErr = Importer->Import_New(FromType))
+return *ImportedOrErr;
+  else {
+llvm::consumeError(ImportedOrErr.takeError());
+return QualType{};
+  }
 }
   };
 
Index: lib/Frontend/ASTMerge.cpp
===
--- lib/Frontend/ASTMerge.cpp
+++ lib/Frontend/ASTMerge.cpp
@@ -65,11 +65,13 @@
   if (II->isStr("__va_list_tag") || II->isStr("__builtin_va_list"))
 continue;
 
-  Decl *ToD = Importer.Import(D);
+  llvm::Expected ToDOrError = Importer.Import_New(D);
 
-  if (ToD) {
-DeclGroupRef DGR(ToD);
+  if (ToDOrError) {
+DeclGroupRef DGR(*ToDOrError);
 CI.getASTConsumer().HandleTopLevelDecl(DGR);
+  } else {
+llvm::consumeError(ToDOrError.takeError());
   }
 }
   }
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -245,11 +245,28 @@
 
 llvm::Expected
 CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) {
-  ASTImporter  = getOrCreateASTImporter(FD->getASTContext());
-  auto *ToDecl =
-  cast(Importer.Import(const_cast(FD)));
-  assert(ToDecl->hasBody());
   

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 175675.
balazske added a comment.
Herald added a reviewer: shafik.
Herald added a subscriber: gamesh411.

Rebase to master.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53757/new/

https://reviews.llvm.org/D53757

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -168,32 +168,20 @@
 // Use this to import pointers of specific type.
 template 
 LLVM_NODISCARD Error importInto(ImportT *, ImportT *From) {
-  auto ToI = Importer.Import(From);
-  if (!ToI && From)
-return make_error();
-  To = cast_or_null(ToI);
-  return Error::success();
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (ToOrErr) {
-  //  To = cast_or_null(*ToOrErr);
-  //}
-  //return ToOrErr.takeError();
+  auto ToOrErr = Importer.Import_New(From);
+  if (ToOrErr)
+To = cast_or_null(*ToOrErr);
+  return ToOrErr.takeError();
 }
 
 // Call the import function of ASTImporter for a baseclass of type `T` and
 // cast the return value to `T`.
 template 
 Expected import(T *From) {
-  auto *To = Importer.Import(From);
-  if (!To && From)
-return make_error();
-  return cast_or_null(To);
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (!ToOrErr)
-  //  return ToOrErr.takeError();
-  //return cast_or_null(*ToOrErr);
+  auto ToOrErr = Importer.Import_New(From);
+  if (!ToOrErr)
+return ToOrErr.takeError();
+  return cast_or_null(*ToOrErr);
 }
 
 template 
@@ -204,13 +192,7 @@
 // Call the import function of ASTImporter for type `T`.
 template 
 Expected import(const T ) {
-  T To = Importer.Import(From);
-  T DefaultT;
-  if (To == DefaultT && !(From == DefaultT))
-return make_error();
-  return To;
-  // FIXME: This should be the final code.
-  //return Importer.Import(From);
+  return Importer.Import_New(From);
 }
 
 template 
@@ -300,8 +282,15 @@
   Importer.MapImported(FromD, 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())
@@ -647,15 +636,6 @@
 FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected ASTNodeImporter::import(const TemplateName ) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-return make_error();
-  return To;
-}
-
 template 
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
 SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
@@ -1693,15 +1673,10 @@
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter ) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-Decl *ToTypedef = Importer.Import(FromTypedef);
-if (!ToTypedef)
-  return make_error();
-To->setTypedefNameForAnonDecl(cast(ToTypedef));
-// FIXME: This should be the final code.
-//if (Expected ToTypedefOrErr = Importer.Import(FromTypedef))
-//  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
-//else
-//  return ToTypedefOrErr.takeError();
+if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
+else
+  return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
@@ -3437,9 +3412,6 @@
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, Szelethus, martong, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: a.sidorin.

Repository:
  rC Clang

https://reviews.llvm.org/D53757

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -187,32 +187,20 @@
 // Use this to import pointers of specific type.
 template 
 LLVM_NODISCARD Error importInto(ImportT *, ImportT *From) {
-  auto ToI = Importer.Import(From);
-  if (!ToI && From)
-return make_error();
-  To = cast_or_null(ToI);
-  return Error::success();
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (ToOrErr) {
-  //  To = cast_or_null(*ToOrErr);
-  //}
-  //return ToOrErr.takeError();
+  auto ToOrErr = Importer.Import_New(From);
+  if (ToOrErr)
+To = cast_or_null(*ToOrErr);
+  return ToOrErr.takeError();
 }
 
 // Call the import function of ASTImporter for a baseclass of type `T` and
 // cast the return value to `T`.
 template 
 Expected import(T *From) {
-  auto *To = Importer.Import(From);
-  if (!To && From)
-return make_error();
-  return cast_or_null(To);
-  // FIXME: This should be the final code.
-  //auto ToOrErr = Importer.Import(From);
-  //if (!ToOrErr)
-  //  return ToOrErr.takeError();
-  //return cast_or_null(*ToOrErr);
+  auto ToOrErr = Importer.Import_New(From);
+  if (!ToOrErr)
+return ToOrErr.takeError();
+  return cast_or_null(*ToOrErr);
 }
 
 template 
@@ -223,13 +211,7 @@
 // Call the import function of ASTImporter for type `T`.
 template 
 Expected import(const T ) {
-  T To = Importer.Import(From);
-  T DefaultT;
-  if (To == DefaultT && !(From == DefaultT))
-return make_error();
-  return To;
-  // FIXME: This should be the final code.
-  //return Importer.Import(From);
+  return Importer.Import_New(From);
 }
 
 template 
@@ -319,8 +301,15 @@
   Importer.MapImported(FromD, 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())
@@ -665,15 +654,6 @@
 FunctionDecl *FromFD);
   };
 
-// FIXME: Temporary until every import returns Expected.
-template <>
-Expected ASTNodeImporter::import(const TemplateName ) {
-  TemplateName To = Importer.Import(From);
-  if (To.isNull() && !From.isNull())
-return make_error();
-  return To;
-}
-
 template 
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
 SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
@@ -1707,15 +1687,10 @@
 static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter ) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
-Decl *ToTypedef = Importer.Import(FromTypedef);
-if (!ToTypedef)
-  return make_error();
-To->setTypedefNameForAnonDecl(cast(ToTypedef));
-// FIXME: This should be the final code.
-//if (Expected ToTypedefOrErr = Importer.Import(FromTypedef))
-//  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
-//else
-//  return ToTypedefOrErr.takeError();
+if (ExpectedDecl ToTypedefOrErr = Importer.Import_New(FromTypedef))
+  To->setTypedefNameForAnonDecl(cast(*ToTypedefOrErr));
+else
+  return ToTypedefOrErr.takeError();
   }
   return Error::success();
 }
@@ -3438,9 +3413,6 @@
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits