[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

2019-03-05 Thread Gabor Marton via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL355390: [ASTImporter] Fix redecl failures of Class and 
ClassTemplate (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58502

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

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2553,26 +2553,6 @@
 Decl::FOK_None;
   }
 
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D &&
-  // Friend template declaration must be imported on its own.
-  !IsFriendTemplate &&
-  // In contrast to a normal CXXRecordDecl, the implicit
-  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
-  // The definition of the implicit CXXRecordDecl in this case is the
-  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
-  // condition in order to be able to import the implict Decl.
-  !D->isImplicit()) {
-ExpectedDecl ImportedDefOrErr = import(Definition);
-if (!ImportedDefOrErr)
-  return ImportedDefOrErr.takeError();
-
-return Importer.MapImported(D, *ImportedDefOrErr);
-  }
-
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -2601,7 +2581,8 @@
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
 if (!FoundDecls.empty()) {
-  // We're going to have to compare D against potentially conflicting Decls, so complete it.
+  // We're going to have to compare D against potentially conflicting Decls,
+  // so complete it.
   if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition())
 D->getASTContext().getExternalSource()->CompleteType(D);
 }
@@ -4976,17 +4957,6 @@
 ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
 
-  // If this template has a definition in the translation unit we're coming
-  // from, but this particular declaration is not that definition, import the
-  // definition and map to that.
-  ClassTemplateDecl *Definition = getDefinition(D);
-  if (Definition && Definition != D && !IsFriend) {
-if (ExpectedDecl ImportedDefOrErr = import(Definition))
-  return Importer.MapImported(D, *ImportedDefOrErr);
-else
-  return ImportedDefOrErr.takeError();
-  }
-
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -4241,8 +4241,7 @@
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4250,16 +4249,14 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportDefinitionAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

2019-03-04 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik accepted this revision.
shafik added a comment.

LGTM, thank you !


Repository:
  rC Clang

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

https://reviews.llvm.org/D58502



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

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

2019-02-28 Thread Gabor Marton via Phabricator via cfe-commits 
martong added a comment.

@shafik, Could you please take a look?
I have run the LLDB tests on our macOS and I could not discover any regression.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58502



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

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

2019-02-24 Thread 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 Gabor,
I don't see any problems with the patch. Thanks! I think it will be good to get 
Shafik's approval as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58502



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

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

2019-02-21 Thread Gabor Marton via Phabricator via cfe-commits 
martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, 
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

Redecl chains of classes and class templates are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.


Repository:
  rC Clang

https://reviews.llvm.org/D58502

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4207,8 +4207,7 @@
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4216,16 +4215,14 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportDefinitionAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportPrototypeAfterImportedDefinition)
@@ -4233,8 +4230,7 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportPrototypeAfterImportedDefinition)
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2552,26 +2552,6 @@
 Decl::FOK_None;
   }
 
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D &&
-  // Friend template declaration must be imported on its own.
-  !IsFriendTemplate &&
-  // In contrast to a normal CXXRecordDecl, the implicit
-  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
-  // The definition of the implicit CXXRecordDecl in this case is the
-  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
-  // condition in order to be able to import the implict Decl.
-  !D->isImplicit()) {
-ExpectedDecl ImportedDefOrErr = import(Definition);
-if (!ImportedDefOrErr)
-  return ImportedDefOrErr.takeError();
-
-return Importer.MapImported(D, *ImportedDefOrErr);
-  }
-
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -2600,7 +2580,8 @@
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
 if (!FoundDecls.empty()) {
-  // We're going to have to compare D against potentially conflicting Decls, so complete it.
+  // We're going to have to compare D against potentially