subject:"\[PATCH\] D144626\: \[C\+\+20\] \[Modules\] Trying to compare the trailing require clause of the primary template when performing ODR checking"

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-03-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6704
+};
+const FunctionDecl *PrimaryX = TryToGetPrimaryTemplatedFunction(FuncX);
+const FunctionDecl *PrimaryY = TryToGetPrimaryTemplatedFunction(FuncY);

If this is necessary here, I'd expect it to be necessary in a lot of other 
places too. (For example, we perform basically the same comparison when doing 
redeclaration checking.) Which declarations have the wrong requires-clause 
attached to them? Why would the requires-clause be modified from the one 
written in the original declaration?

(I could imagine this happening for explicit instantiations, where the requires 
clause might not be specified at all, but we could address that by looking at 
the canonical declaration for a trailing requires-clause, rather than looking 
at the template pattern.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-28 Thread Chuanqi Xu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e50578ba43c: [C++20] [Modules] Trying to compare the 
trailing require clause from the… (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144626

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/pr60890.cppm

Index: clang/test/Modules/pr60890.cppm
===
--- /dev/null
+++ 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
+struct a {
+	friend void aa(a x) requires(true) {}
+	void aaa() requires(true) {}
+};
+
+export template struct a;
+
+export template
+void foo(T) requires(true) {}
+
+export template void foo(double);
+
+export template 
+class A {
+	friend void foo<>(A);
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+void b() {
+a _;
+	a __;
+}
+
+//--- c.cppm
+export module c;
+
+import a;
+
+struct c {
+	void f() const {
+		a _;
+		aa(_);
+		_.aaa();
+
+		a __;
+		aa(__);
+		__.aaa();
+
+		foo(5);
+		foo(3.0);
+		foo(A());
+	}
+};
+
+//--- d.cpp
+// expected-no-diagnostics
+import a;
+import b;
+import c;
+
+void d() {
+	a _;
+	aa(_);
+	_.aaa();
+
+	a __;
+	aa(__);
+	__.aaa();
+
+	foo(5);
+	foo(3.0);
+	foo(A());
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6683,8 +6683,28 @@
 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;
 
 auto GetTypeAsWritten = [](const FunctionDecl *FD) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-28 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

In D144626#4158290 , @erichkeane 
wrote:

> In D144626#4157240 , @ChuanqiXu 
> wrote:
>
>> This is another revision (https://reviews.llvm.org/D144707) which shouldn't 
>> be related with libcxx's modular build. So the failures should be irrelevant 
>> with the revision. @erichkeane @royjacobson Do you think it makes sense to 
>> land this revision in this case?
>
> Ah, sorry, I meant to come back to this and let you know...  The libcxx 
> modules builds are broken for an unknown reason unrelated to this.  Feel free 
> to land this.

Thanks~


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-28 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D144626#4157240 , @ChuanqiXu wrote:

> This is another revision (https://reviews.llvm.org/D144707) which shouldn't 
> be related with libcxx's modular build. So the failures should be irrelevant 
> with the revision. @erichkeane @royjacobson Do you think it makes sense to 
> land this revision in this case?

Ah, sorry, I meant to come back to this and let you know...  The libcxx modules 
builds are broken for an unknown reason unrelated to this.  Feel free to land 
this.


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-27 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

This is another revision (https://reviews.llvm.org/D144707) which shouldn't be 
related with libcxx's modular build. So the failures should be irrelevant with 
the revision. @erichkeane @royjacobson Do you think it makes sense to land this 
revision in this case?


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-27 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu updated this revision to Diff 500703.
ChuanqiXu added a comment.

Rebase and push again to trigger the bot.


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

https://reviews.llvm.org/D144626

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/pr60890.cppm

Index: clang/test/Modules/pr60890.cppm
===
--- /dev/null
+++ 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
+struct a {
+	friend void aa(a x) requires(true) {}
+	void aaa() requires(true) {}
+};
+
+export template struct a;
+
+export template
+void foo(T) requires(true) {}
+
+export template void foo(double);
+
+export template 
+class A {
+	friend void foo<>(A);
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+void b() {
+a _;
+	a __;
+}
+
+//--- c.cppm
+export module c;
+
+import a;
+
+struct c {
+	void f() const {
+		a _;
+		aa(_);
+		_.aaa();
+
+		a __;
+		aa(__);
+		__.aaa();
+
+		foo(5);
+		foo(3.0);
+		foo(A());
+	}
+};
+
+//--- d.cpp
+// expected-no-diagnostics
+import a;
+import b;
+import c;
+
+void d() {
+	a _;
+	aa(_);
+	_.aaa();
+
+	a __;
+	aa(__);
+	__.aaa();
+
+	foo(5);
+	foo(3.0);
+	foo(A());
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6683,8 +6683,28 @@
 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;
 
 auto GetTypeAsWritten = [](const FunctionDecl *FD) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-27 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

In D144626#4154122 , @ChuanqiXu wrote:

> In D144626#4150378 , @erichkeane 
> wrote:
>
>> That looks reasonable to me, though I fear all the libcxx failures are going 
>> to be related to this, please make sure to check them out!
>
> I tested locally that the libcxx failures are not related to  this patch 
> after I reverted this change.

Maybe you can rebase and push a change to trigger the bots again.


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

In D144626#4150378 , @erichkeane 
wrote:

> That looks reasonable to me, though I fear all the libcxx failures are going 
> to be related to this, please make sure to check them out!

I tested locally that the libcxx failures are not related to  this patch after 
I reverted this change.


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-24 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

That looks reasonable to me, though I fear all the libcxx failures are going to 
be related to this, please make sure to check them out!


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

2023-02-23 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

Yeah, it looks better indeed to not store additional information.


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

https://reviews.llvm.org/D144626

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

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

[PATCH] D144626: [C++20] [Modules] Trying to compare the trailing require clause of the primary template when performing ODR checking

10 matches

Site Navigation

Mail list logo

Footer information