[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368163: [ASTImporter] Do not import FunctionTemplateDecl in 
record twice. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65203?vs=211743&id=213858#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65203

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
@@ -3118,9 +3118,19 @@
   if (FoundByLookup) {
 if (isa(FoundByLookup)) {
   if (D->getLexicalDeclContext() == D->getDeclContext()) {
-if (!D->doesThisDeclarationHaveABody())
+if (!D->doesThisDeclarationHaveABody()) {
+  if (FunctionTemplateDecl *DescribedD =
+  D->getDescribedFunctionTemplate()) {
+// Handle a "templated" function together with its described
+// template. This avoids need for a similar check at import of the
+// described template.
+assert(FoundByLookup->getDescribedFunctionTemplate() &&
+   "Templated function mapped to non-templated?");
+Importer.MapImported(DescribedD,
+ 
FoundByLookup->getDescribedFunctionTemplate());
+  }
   return Importer.MapImported(D, FoundByLookup);
-else {
+} else {
   // Let's continue and build up the redecl chain in this case.
   // FIXME Merge the functions into one decl.
 }
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -2389,6 +2389,49 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionTemplateWithDefInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  template 
+  void X::f(T t) {};
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5223,6 +5266,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctions,
 DefaultTestValuesForRunOptions, );
 


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -3118,9 +3118,19 @@
   if (FoundByLookup) {
 if (isa(FoundByLookup)) {
   if (D->getLexicalDeclContext() == D->getDeclContext()) {
-if (!D->doesThisDeclarationHaveABody())
+if (!D->doesThisDeclarationHaveABody()) {
+  if (FunctionTemplateDecl *DescribedD =
+  D->getDescribedFunctionTemplate()) {
+// Handle a "templated" function together with its described
+// template. This avoids need for a similar check at import of the
+// described template.
+assert(FoundByLookup->getDescribedFunctionTemplate() &&
+   "Templated function mapped to non-templated?");
+Importer.MapImported(DescribedD,
+ FoundByLookup->getDescribedFunctionTemplate());

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65203



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

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@a_sidorin This is a polite Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65203



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

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

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

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

Added missing test fixture, removed not a related change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65203

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


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -2389,6 +2389,49 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionTemplateWithDefInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  template 
+  void X::f(T t) {};
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5213,6 +5256,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctions,
 DefaultTestValuesForRunOptions, );
 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3066,9 +3066,19 @@
   if (FoundByLookup) {
 if (isa(FoundByLookup)) {
   if (D->getLexicalDeclContext() == D->getDeclContext()) {
-if (!D->doesThisDeclarationHaveABody())
+if (!D->doesThisDeclarationHaveABody()) {
+  if (FunctionTemplateDecl *DescribedD =
+  D->getDescribedFunctionTemplate()) {
+// Handle a "templated" function together with its described
+// template. This avoids need for a similar check at import of the
+// described template.
+assert(FoundByLookup->getDescribedFunctionTemplate() &&
+   "Templated function mapped to non-templated?");
+Importer.MapImported(DescribedD,
+ 
FoundByLookup->getDescribedFunctionTemplate());
+  }
   return Importer.MapImported(D, FoundByLookup);
-else {
+} else {
   // Let's continue and build up the redecl chain in this case.
   // FIXME Merge the functions into one decl.
 }


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -2389,6 +2389,49 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionTemplateWithDefInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  template 
+  void X::f(T t) {};
+  )";
+

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

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

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

For functions there is a check to not duplicate the declaration if it is in a
record (class). For function templates there was no similar check, if a
template (in the same class) was imported multiple times the
FunctionTemplateDecl was created multiple times with the same templated
FunctionDecl. This can result in problems with the declaration chain.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65203

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


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -2389,6 +2389,47 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionTemplateWithDefInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  template 
+  void X::f(T t) {};
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher().match(
+  FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3066,9 +3066,19 @@
   if (FoundByLookup) {
 if (isa(FoundByLookup)) {
   if (D->getLexicalDeclContext() == D->getDeclContext()) {
-if (!D->doesThisDeclarationHaveABody())
+if (!D->doesThisDeclarationHaveABody()) {
+  if (FunctionTemplateDecl *DescribedD =
+  D->getDescribedFunctionTemplate()) {
+// Handle a "templated" function together with its described
+// template. This avoids need for a similar check at import of the
+// described template.
+assert(FoundByLookup->getDescribedFunctionTemplate() &&
+   "Templated function mapped to non-templated?");
+Importer.MapImported(DescribedD,
+ 
FoundByLookup->getDescribedFunctionTemplate());
+  }
   return Importer.MapImported(D, FoundByLookup);
-else {
+} else {
   // Let's continue and build up the redecl chain in this case.
   // FIXME Merge the functions into one decl.
 }
@@ -5550,14 +5560,14 @@
 }
   }
 
-  auto ParamsOrErr = import(D->getTemplateParameters());
-  if (!ParamsOrErr)
-return ParamsOrErr.takeError();
-
   FunctionDecl *TemplatedFD;
   if (Error Err = importInto(TemplatedFD, D->getTemplatedDecl()))
 return std::move(Err);
 
+  auto ParamsOrErr = import(D->getTemplateParameters());
+  if (!ParamsOrErr)
+return ParamsOrErr.takeError();
+
   FunctionTemplateDecl *ToFunc;
   if (GetImportedOrCreateDecl(ToFunc, D, Importer.getToContext(), DC, Loc, 
Name,
   *ParamsOrErr, TemplatedFD))


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -2389,6 +2389,47 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+  R"(
+  class X {
+template 
+void f(T t);
+  };
+  )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc"