[clang] 2408f97 - Recommit [C++20] [Modules] Trying to compare the trailing require clause from the primary template function

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-03T17:27:37+08:00
New Revision: 2408f97652caadf733a879e1d7d9c0702a80d609

URL: https://github.com/llvm/llvm-project/commit/2408f97652caadf733a879e1d7d9c0702a80d609
DIFF: https://github.com/llvm/llvm-project/commit/2408f97652caadf733a879e1d7d9c0702a80d609.diff

LOG: Recommit [C++20] [Modules] Trying to compare the trailing require clause from the primary template function

Close https://github.com/llvm/llvm-project/issues/60890.

For the following example:

```
export module a;

export template<typename T>
struct a {
	friend void aa(a) requires(true) {
	}
};
```

```
export module b;

import a;

struct b {
	a<int> m;
};
```

```
export module c;

import a;

struct c {
	void f() const {
		aa(a<int>());
	}
};
```

```
import a;
import b;
import c;

void d() {
	aa(a<int>());
}
```

The current clang will reject this incorrectly. The reason is that the
require clause  will be replaced with the evaluated version
(https://github.com/llvm/llvm-project/blob/efae3174f09560353fb0f3d528bcbffe060d5438/clang/lib/Sema/SemaConcept.cpp#L664-L665).
In module 'b', the friend function is instantiated but not used so the
require clause of the friend function is `(true)`. However, in module
'c', the friend function is used so the require clause is `true`. So
deserializer classify these two function to two different functions
instead of one. Then here is the bug report.

The proposed solution is to try to compare the trailing require clause
of the primary template when performing ODR checking.

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D144626

Added: 
    clang/test/Modules/pr60890.cppm

Modified: 
    clang/lib/AST/ASTContext.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 482791ada5d1..f22683296664 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6591,7 +6591,10 @@ bool ASTContext::FriendsDifferByConstraints(const FunctionDecl *X,
 
   // If the two functions share lexical declaration context, they are not in
   // separate instantations, and thus in the same scope.
-  if (X->getLexicalDeclContext() == Y->getLexicalDeclContext())
+  if (declaresSameEntity(cast<Decl>(X->getLexicalDeclContext()
+                             ->getRedeclContext()),
+                         cast<Decl>(Y->getLexicalDeclContext()
+                             ->getRedeclContext())))
     return false;
 
   if (!X->getDescribedFunctionTemplate()) {
@@ -6683,8 +6686,28 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
         return false;
     }
 
-    if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
-                              FuncY->getTrailingRequiresClause()))
+    // The trailing require clause of instantiated function may change during
+    // the semantic analysis. Trying to get the primary template function (if
+    // exists) to compare the primary trailing require clause.
+    auto TryToGetPrimaryTemplatedFunction =
+        [](const FunctionDecl *FD) -> const FunctionDecl * {
+      switch (FD->getTemplatedKind()) {
+      case FunctionDecl::TK_DependentNonTemplate:
+        return FD->getInstantiatedFromDecl();
+      case FunctionDecl::TK_FunctionTemplate:
+        return FD->getDescribedFunctionTemplate()->getTemplatedDecl();
+      case FunctionDecl::TK_MemberSpecialization:
+        return FD->getInstantiatedFromMemberFunction();
+      case FunctionDecl::TK_FunctionTemplateSpecialization:
+        return FD->getPrimaryTemplate()->getTemplatedDecl();
+      default:
+        return FD;
+      }
+    };
+    const FunctionDecl *PrimaryX = TryToGetPrimaryTemplatedFunction(FuncX);
+    const FunctionDecl *PrimaryY = TryToGetPrimaryTemplatedFunction(FuncY);
+    if (!isSameConstraintExpr(PrimaryX->getTrailingRequiresClause(),
+                              PrimaryY->getTrailingRequiresClause()))
       return false;
 
     // Constrained friends are 
diff erent in certain cases, see: [temp.friend]p9.

diff  --git a/clang/test/Modules/pr60890.cppm b/clang/test/Modules/pr60890.cppm
new file mode 100644
index 000000000000..2560bec5b433
--- /dev/null
+++ b/clang/test/Modules/pr60890.cppm
@@ -0,0 +1,82 @@
+// https://github.com/llvm/llvm-project/issues/60890
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -fprebuilt-module-path=%t -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/c.cppm -fprebuilt-module-path=%t -o %t/c.pcm
+// RUN: %clang_cc1 -std=c++20 %t/d.cpp -fprebuilt-module-path=%t -S -emit-llvm -o -
+
+//--- a.cppm
+export module a;
+ 
+export template<typename T>
+struct a {
+	friend void aa(a x) requires(true) {}
+	void aaa() requires(true) {}
+};
+
+export template struct a<double>;
+
+export template<typename T>
+void foo(T) requires(true) {}
+
+export template void foo<double>(double);
+
+export template <typename T>
+class A {
+	friend void foo<>(A);
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+void b() {
+    a<int> _;
+	a<double> __;
+}
+
+//--- c.cppm
+export module c;
+
+import a;
+
+struct c {
+	void f() const {
+		a<int> _;
+		aa(_);
+		_.aaa();
+
+		a<double> __;
+		aa(__);
+		__.aaa();
+
+		foo<int>(5);
+		foo<double>(3.0);
+		foo(A<int>());
+	}
+};
+
+//--- d.cpp
+// expected-no-diagnostics
+import a;
+import b;
+import c;
+
+void d() {
+	a<int> _;
+	aa(_);
+	_.aaa();
+
+	a<double> __;
+	aa(__);
+	__.aaa();
+
+	foo<int>(5);
+	foo<double>(3.0);
+	foo(A<int>());
+}