ping～ @donat.nagy @steakhal @balazske

update according to llvm convention.

In D159479#4640590, @donat.nagy wrote:

Today we learned that truth is different from falsehood...

LGTM, nice catch!

In D159479#4640607, @steakhal wrote:

Make sure the commit message complies with the guidelines.

In D159412#4636813, @donat.nagy wrote:

Please add a testcase that demonstrates this issue (fails when your change in MemRegion.cpp isn't added) and shows that your commit fixes it.

add argument comment according to bugprone-argument-comment.

In D156693#4603537, @balazske wrote:

My concern was related to nested namespaces or nested classes with friend declarations that are equivalent and differ only in the nesting level. It may be possible to construct code where a declaration at an inner (nested) level is found to be equivalent with a similar looking class at an outer level. But I now do not have time to look for an example to test it, and I am not fully familiar with exact rules of friend declarations, so I accept this fix.

In D156693#4584657, @balazske wrote:

A simple test should be added to StructuralEquivalenceTest.cpp too to check if ignore (and not ignore) depth works.

I think this solution is not always correct, but is still an improvement.

Fix variable names according to the convention.

Add testcase to illustrate the difference between TemplateTypeParm and NonTypeTemplateParm in detecting by structural equivalence.

Add more simple testcase to StructuralEquivalenceTest.cpp.

fixed according to the review and add testcase to StructuralEquivalenceTest.cpp.

In D156693#4568941, @balazske wrote:

This fix can cause problems because the depth comparison is omitted all times. It would be better to omit depth check if the imported template is a friend, and has a ClassTemplateSpecializationDecl as context. Probably even then it is possible to construct cases where the checked template has references to other templates with different "depth" which should not omitted in the check. But I have no idea of a better solution, only to pass a ClassTemplateDecl or ClassTemplateSpecializationDecl to StructuralEquivalenceContext and omit the depth check only at this object.

update diff: add more conditions on when to ignore comparing depth of two template classes to minimize influence to others and fix name of test case to make easy understanding

clean unittest

update diff: ignore comparing depth in friend class template

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 T, T U>
class A;

template<class T, T U>
class A {
public:
  template<class P, P Q>
  friend class A;

  A(T x):x(x){}
	
private:
  T x;
};

A<int,3> 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.

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?

add unittest

format code

update patch for passing unittest and test

update patch for passing unittest

update patch