[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky updated this revision to Diff 547501.
jcsxky edited the summary of this revision.
jcsxky added a comment.

update diff: ignore compare depth in friend class template


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/include/clang/AST/ASTStructuralEquivalence.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4212,6 +4212,57 @@
   EXPECT_TRUE(Imported->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses, SkipFriendTemplateDeclaration) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  class A;
+
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x)  :x(x) {}
+
+  private:
+T x;
+  };
+  )",
+  Lang_CXX11);
+
+  auto *Fwd = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("A")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  class A;
+
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x) : x(x) {}
+
+  private:
+T x;
+  };
+
+  A a1(0);
+  )",
+  Lang_CXX11, "input1.cc");
+  auto *Definition = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("A")));
+  auto *Template = Import(Definition, Lang_CXX11);
+  EXPECT_TRUE(Template);
+  auto *TemplateClass = cast(Template);
+  EXPECT_EQ(Fwd->getTemplatedDecl()->getTypeForDecl(),
+TemplateClass->getTemplatedDecl()->getTypeForDecl());
+}
+
 TEST_P(ImportFriendClasses,
ImportOfClassTemplateDefinitionShouldConnectToFwdFriend) {
   Decl *ToTU = getToTuDecl(
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1092,7 +1092,7 @@
   case Type::TemplateTypeParm: {
 const auto *Parm1 = cast(T1);
 const auto *Parm2 = cast(T2);
-if (Parm1->getDepth() != Parm2->getDepth())
+if (!Context.IgnoreDepth && Parm1->getDepth() != Parm2->getDepth())
   return false;
 if (Parm1->getIndex() != Parm2->getIndex())
   return false;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -34,6 +34,7 @@
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
@@ -506,7 +507,8 @@
 template 
 bool hasSameVisibilityContextAndLinkage(T *Found, T *From);
 
-bool IsStructuralMatch(Decl *From, Decl *To, bool Complain = true);
+bool IsStructuralMatch(Decl *From, Decl *To, bool Complain = true,
+   bool IgnoreDepth = true);
 ExpectedDecl VisitDecl(Decl *D);
 ExpectedDecl VisitImportDecl(ImportDecl *D);
 ExpectedDecl VisitEmptyDecl(EmptyDecl *D);
@@ -2243,7 +2245,8 @@
 : StructuralEquivalenceKind::Default;
 }
 
-bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain) {
+bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain,
+bool IgnoreDepth) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToRecord.
   Decl *ToOrigin = Importer.GetOriginalDecl(To);
@@ -2254,7 +2257,7 @@
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, Complain);
+  false, Complain, false, IgnoreDepth);
   return Ctx.IsEquivalent(From, To);
 }
 
@@ -5822,7 +5825,7 @@
 if (!hasSameVisibilityContextAndLinkage(FoundTemplate, D))
   continue;
 
-if (IsStructuralMatch(D, FoundTemplate)) {
+if (IsStructuralMatch(D, FoundTemplate, true, true)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
   if (D->isThisDeclarationADefinition() && TemplateWithDef)
Index: clang/include/clang/AST/ASTStructuralEquivalence.h
===
--- clang/include/clang/AST/ASTStructuralEquivalence.h
+++ clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -69,15 +69,19 @@
   /// \c true if the last diagnostic came from ToCtx.
   bool LastDiagFromC2 = false;
 
+  /// Whether to igno

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky added a comment.

In D156693#4560110 , @balazske wrote:

> The summary tells nothing about what is the real problem to fix here. In the 
> lit test I see that an error message is displayed, this causes the test 
> failure. The problem is that the structural equivalence check is not good for 
> this special case. The imported AST contains a class template specialization 
> for `A`, in this specialization the friend class template `A` has a previous 
> decl that points to the original friend class template that is in a 
> `ClassTemplateDecl`. In the specialization the "depth" of template arguments 
> is 0, but in the original template it is 1 (the "to" code at import contains 
> only the "original template", no specialization). This difference in the 
> "depth" causes the type mismatch when the specialization is imported.
> AST dump of this code can show the problem:
>
>   template
>   class A;
>   
>   template
>   class A {
>   public:
> template
> friend class A;
>   
> A(T x):x(x){}
>   
>   private:
> T x;
>   };
>   
>   A a1(0);
>
> It is really interesting that at friend templates the depth is 1 but friend 
> declarations point to objects outside the class, so really the depth should 
> not increase in a friend template from this point of view. But this is an AST 
> issue.

Depth of specialization friend template is 0 while it is 1 in class declaration 
 is reasonable, because after template class specialization there not exists 
template parameter and the friend template becomes outermost scope, thus depth 
is 0. Thanks to the different of depth causes the non equivalence of the two 
'NonTypeTemplateParm', I think there should ignore comparing the depth in this 
special case to make two types equivalence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

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

balazske added a comment.

The summary tells nothing about what is the real problem to fix here. In the 
lit test I see that an error message is displayed, this causes the test 
failure. The problem is that the structural equivalence check is not good for 
this special case. The imported AST contains a class template specialization 
for `A`, in this specialization the friend class template `A` has a previous 
decl that points to the original friend class template that is in a 
`ClassTemplateDecl`. In the specialization the "depth" of template arguments is 
0, but in the original template it is 1 (the "to" code at import contains only 
the "original template", no specialization). This difference in the "depth" 
causes the type mismatch when the specialization is imported.
AST dump of this code can show the problem:

  template
  class A;
  
  template
  class A {
  public:
template
friend class A;
  
A(T x):x(x){}

  private:
T x;
  };
  
  A a1(0);

It is really interesting that at friend templates the depth is 1 but friend 
declarations point to objects outside the class, so really the depth should not 
increase in a friend template from this point of view. But this is an AST issue.




Comment at: clang/lib/AST/ASTImporter.cpp:5829
+!IsStructuralMatch(D, FoundTemplate, false))
+  continue;
 if (IsStructuralMatch(D, FoundTemplate)) {

It is not good to use `ParentMap` in the AST importer because it does AST 
traversal, even worse if this is done on the To context where the AST is 
modified and may be in incomplete state.
This way of fix is probably not good for a case when there is a real structural 
in-equivalence, this would be not detected. And the current solution skips this 
`FoundDecl` but (at least in the used test code) this should be found, not 
skipped. (But we can create code where the skip is correct, if there is a real 
structural in-equivalence.)




Comment at: clang/unittests/AST/ASTImporterTest.cpp:4218
+  R"(
+namespace __1{
+

I think the `namespace __1` is not important for reproduction of this problem.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4234
+  int j=1/i;
+  (void)j;
+}

Functions `foo`, `bar`, `main` are not required. It is only important to have a 
variable of type `A` like `A a1(0);` in the imported code at getTuDecl.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4247
+  }
+  )",
+  Lang_CXX11);

The coding format should be aligned to the format of other test codes in this 
file, and this is normally same as the clang format guidelines (automatic 
reformatting does not work in the test code).



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4284
+  Lang_CXX11, "input1.cc");
+  auto *Definition = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("A")));

`Definition` is misleading because this is not the definition, it matches the 
first declaration of `A` in the AST. Better name is like `FromA` like in the 
other tests, or FromXxx.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4286
+  FromTU, classTemplateDecl(hasName("A")));
+  auto *Template = Import(Definition, Lang_CXX11);
+  EXPECT_TRUE(Template);

The imported name can be `ToA` or ToXxx or ImportedXxx, this makes real 
distinction between the from and to objects.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4288
+  EXPECT_TRUE(Template);
+  auto *TemplateClass = cast(Template);
+  EXPECT_EQ(Fwd->getTemplatedDecl()->getTypeForDecl(),

This cast is not needed, type of `Template` is already `ClassTemplateDecl*`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky added a comment.

In D156693#4560023 , @Michael137 
wrote:

> Could you please elaborate in the commit message what exactly the issue 
> currently is and how the patch addresses it?



In D156693#4560023 , @Michael137 
wrote:

> Could you please elaborate in the commit message what exactly the issue 
> currently is and how the patch addresses it?

details are added to commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

Could you please elaborate in the commit message what exactly the issue 
currently is and how the patch addresses it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky updated this revision to Diff 547124.
jcsxky added a comment.

add unittest


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
  clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
  clang/test/Analysis/Inputs/ctu-friend-template.h
  clang/test/Analysis/ctu-friend-template.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4212,6 +4212,84 @@
   EXPECT_TRUE(Imported->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses, SkipFriendTemplateDeclaration) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+namespace __1{
+
+  template
+  class A;
+
+  template
+  class A{
+  public:
+template
+friend class A;
+
+A(T x):x(x){}
+
+void foo(){
+  int i=0;
+  int j=1/i;
+  (void)j;
+}
+
+  private:
+T x;
+  };
+
+  }
+  void bar();
+
+  int main(){
+bar();
+  }
+  )",
+  Lang_CXX11);
+
+  auto *Fwd = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("A")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace __1{
+
+  template
+  class A;
+
+  template
+  class A{
+  public:
+template
+friend class A;
+
+A(T x):x(x){}
+
+void foo(){
+  int i=0;
+  int j=1/i;
+  (void)j;
+}
+
+  private:
+T x;
+  };
+
+}
+void bar(){
+  __1::A a1(0);
+  a1.foo();
+}
+  )",
+  Lang_CXX11, "input1.cc");
+  auto *Definition = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("A")));
+  auto *Template = Import(Definition, Lang_CXX11);
+  EXPECT_TRUE(Template);
+  auto *TemplateClass = cast(Template);
+  EXPECT_EQ(Fwd->getTemplatedDecl()->getTypeForDecl(),
+TemplateClass->getTemplatedDecl()->getTypeForDecl());
+}
+
 TEST_P(ImportFriendClasses,
ImportOfClassTemplateDefinitionShouldConnectToFwdFriend) {
   Decl *ToTU = getToTuDecl(
Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast %S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+	bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+	template
+	friend class A;
+
+	A(T x):x(x){}
+	
+	void foo(){}
+	
+private:
+	T x;
+};
+
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
@@ -0,0 +1 @@
+9:c:@F@bar# ctu-friend-template-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
@@ -0,0 +1,6 @@
+#include "ctu-friend-template.h"
+
+void bar(){
+	__1::A a1(0);
+	a1.foo();
+}
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -34,6 +34,7 @@
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
@@ -5821,7 +5822,11 @@
   if (FoundTemplate) {

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky updated this revision to Diff 546698.
jcsxky added a comment.

format code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
  clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
  clang/test/Analysis/Inputs/ctu-friend-template.h
  clang/test/Analysis/ctu-friend-template.cpp


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast 
%S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt 
%t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+   bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+   template
+   friend class A;
+
+   A(T x):x(x){}
+   
+   void foo(){}
+   
+private:
+   T x;
+};
+
+}
Index: 
clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
@@ -0,0 +1 @@
+9:c:@F@bar# ctu-friend-template-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
@@ -0,0 +1,6 @@
+#include "ctu-friend-template.h"
+
+void bar(){
+   __1::A a1(0);
+   a1.foo();
+}
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -34,6 +34,7 @@
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
@@ -5821,7 +5822,11 @@
   if (FoundTemplate) {
 if (!hasSameVisibilityContextAndLinkage(FoundTemplate, D))
   continue;
-
+auto Parents = FoundDecl->getASTContext().getParents(*FoundDecl);
+if (!Parents.empty() && nullptr != Parents.begin()->get() 
&&
+FoundTemplate->getName() == D->getName() &&
+!IsStructuralMatch(D, FoundTemplate, false))
+  continue;
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast %S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+	bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+	template
+	friend class A;
+
+	A(T x):x(x){}
+	
+	void foo(){}
+	
+private:
+	T x;
+};
+
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
===

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky updated this revision to Diff 546309.
jcsxky added a comment.

update patch for pass unittests and test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
  clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
  clang/test/Analysis/Inputs/ctu-friend-template.h
  clang/test/Analysis/ctu-friend-template.cpp


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast 
%S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt 
%t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+   bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+   template
+   friend class A;
+
+   A(T x):x(x){}
+   
+   void foo(){}
+   
+private:
+   T x;
+};
+
+}
Index: 
clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
@@ -0,0 +1 @@
+9:c:@F@bar# ctu-friend-template-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
@@ -0,0 +1,6 @@
+#include "ctu-friend-template.h"
+
+void bar(){
+   __1::A a1(0);
+   a1.foo();
+}
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -44,6 +44,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
@@ -5812,6 +5813,11 @@
   if (FoundTemplate) {
 if (!hasSameVisibilityContextAndLinkage(FoundTemplate, D))
   continue;
+auto Parents = FoundDecl->getASTContext().getParents(*FoundDecl);
+if (!Parents.empty() && nullptr != Parents.begin()->get() 
&&
+FoundTemplate->getName() == D->getName() &&
+!IsStructuralMatch(D, FoundTemplate, false))
+  continue;
 
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast %S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+	bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+	template
+	friend class A;
+
+	A(T x):x(x){}
+	
+	void foo(){}
+	
+private:
+	T x;
+};
+
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
==

[PATCH] D156693: [clang][ASTImporter]Skip check friend template declaration in VisitClassTemplateDecl

[Qizhi Hu via Phabricator via cfe-commits]

jcsxky updated this revision to Diff 546301.
jcsxky added a comment.

update patch for pass unittests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
  clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
  clang/test/Analysis/Inputs/ctu-friend-template.h
  clang/test/Analysis/ctu-friend-template.cpp


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast 
%S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt 
%t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+   bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+   template
+   friend class A;
+
+   A(T x):x(x){}
+   
+   void foo(){}
+   
+private:
+   T x;
+};
+
+}
Index: 
clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt
@@ -0,0 +1 @@
+9:c:@F@bar# ctu-friend-template-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template-other.cpp
@@ -0,0 +1,6 @@
+#include "ctu-friend-template.h"
+
+void bar(){
+   __1::A a1(0);
+   a1.foo();
+}
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -44,6 +44,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
@@ -5812,7 +5813,6 @@
   if (FoundTemplate) {
 if (!hasSameVisibilityContextAndLinkage(FoundTemplate, D))
   continue;
-
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
@@ -5824,6 +5824,11 @@
   // see ASTTests test ImportExistingFriendClassTemplateDef.
   continue;
 }
+auto Parents = FoundDecl->getASTContext().getParents(*FoundDecl);
+if (!Parents.empty() && nullptr != Parents.begin()->get() 
&&
+FoundTemplate->getName() == D->getName()) {
+  continue;
+}
 ConflictingDecls.push_back(FoundDecl);
   }
 }


Index: clang/test/Analysis/ctu-friend-template.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-friend-template.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-friend-template-other.cpp.ast %S/Inputs/ctu-friend-template-other.cpp
+// RUN: cp %S/Inputs/ctu-friend-template.cpp.externalDefMap-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// CHECK: CTU loaded AST file
+
+#include "Inputs/ctu-friend-template.h"
+
+void bar();
+
+int main(){
+	bar(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/Inputs/ctu-friend-template.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-friend-template.h
@@ -0,0 +1,20 @@
+namespace __1{
+
+template
+class A;
+
+template
+class A{
+public:
+	template
+	friend class A;
+
+	A(T x):x(x){}
+