[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/mizvekov approved this pull request. Thanks! LGTM. Also don't forget the changeling entry. https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+template requires ((sizeof(T) > 0) && ...) void f() {} //
expected-note{{previous definition is here}}
+class A;
+void operator&&(A, A);
+template requires ((sizeof(T) > 0) && ...) void f() {} //
expected-error{{redefinition of 'f'}}
mizvekov wrote:
Our convention for such tests is to name them as the GitHub issue.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
ChuanqiXu9 wrote: Thanks for the review : ) I'd like to update the release note seperately in a summary before the releasing if necessary. This will be more concrete and easier for backporting https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
mizvekov wrote:
But the bug has nothing to do with modules.
Surely we do not want to replicate or retest every test case as a modules test?
Our modus operandi here is to take a reproducer test from a bug report, and
simplify it as much as possible.
If we take a bug report where the reproducer is a module test, but the bug has
nothing to do with modules, we would just simplify that away.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/195983
>From c54b8fcb7d7760929a92599a1e08d7342c24373e Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 7 Apr 2026 15:04:49 +0800
Subject: [PATCH 1/2] Reland [C++20] [Modules] Don't profiling the callee of
CXXFoldExpr (#190732)
Close https://github.com/llvm/llvm-project/issues/190333
For the test case, the root cause of the problem is, the compiler
thought the declaration of `operator &&` in consumer.cpp may change the
meaning of '&&' in the requrie clause of `F::operator()`. But it doesn't
make sense. Here we skip profiling the callee to solve the problem. Note
that we've already record the kind of the operator. So '&&' and '||'
won't be confused.
---
See the discussion in https://github.com/llvm/llvm-project/pull/194283
For the new found pattern that we may have other binary operator (e.g.,
operator +) in the require clause, e.g.,
```C++
template
requires requires(T t, U u) { t + u; }
void operator()(T, U) {}
```
This is a new problem and we need to solve it in other PR.
---
clang/lib/AST/StmtProfile.cpp | 32 -
.../callable-require-clause-merge.cppm| 35 +++
clang/test/Modules/polluted-operator.cppm | 7
.../diagnose-redefinition-fold-expr.cpp | 6
4 files changed, 72 insertions(+), 8 deletions(-)
create mode 100644 clang/test/Modules/callable-require-clause-merge.cppm
create mode 100644 clang/test/SemaCXX/diagnose-redefinition-fold-expr.cpp
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 8219e57644be6..c3bdcb9a2e60d 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ID.AddInteger(S->getOperator());
}
diff --git a/clang/test/Modules/callable-require-clause-merge.cppm
b/clang/test/Modules/callable-require-clause-merge.cppm
new file mode 100644
index 0..ae49dd7a58542
--- /dev/null
+++ b/clang/test/Modules/callable-require-clause-merge.cppm
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-reduced-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-reduced-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+
+//--- r.h
+struct F {
+ template requires ((sizeof(T) > 0) && ...)
+ void operator()(T...) {}
+} f;
+
+//--- mymod.cppm
+module;
+#include "r.h"
+export module mymod;
+export using ::f;
+
+//--- consumer.cpp
+// expected-no-diagnostics
+void operator&&(struct X, struct X);
+#include "r.h"
+import mymod;
+
+void g() { f(); }
diff --git a/clang/test/Modules/polluted-operator.cppm
b/clang/test/Modules/polluted-operator.cppm
index 45cc5e37d6a64..e81a63c3e03de 100644
--- a/clang/test/Modules/polluted-operator.cppm
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -49,8 +49,6 @@ namespace std
//--- a.cppm
module;
-// The operator&& defined in 'foo.h' will pollute the
-// expression '__is_trivial(_Types) && ...' in bar.h
#include "foo.h"
#include "bar.h"
export module a;
@@ -71,9 +69,4 @@ export namespace std {
using std::operator&&;
}
-#ifdef SKIP_ODR_CHECK_IN_GMF
// expected-no-diagnostics
-#else
-// expected-error@* {{has different definitions in different modules; first
difference is defined here
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
mizvekov wrote:
They are both testing the same polarity, two declarations which should be
equivalent, but are treated as not equivalent by mistake.
The opposite polarity, ie two declarations which should not be equivalent, but
are treated equivalent by mistake, is just not possible with this bug.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -71,9 +69,4 @@ export namespace std {
using std::operator&&;
}
-#ifdef SKIP_ODR_CHECK_IN_GMF
// expected-no-diagnostics
-#else
-// expected-error@* {{has different definitions in different modules; first
difference is defined here found data member '_S_copy_ctor' with an
initializer}}
-// expected-note@* {{but in 'a.' found data member '_S_copy_ctor' with
a different initializer}}
-#endif
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ChuanqiXu9 wrote:
Done, I've updated the comment.
I don't think the module tests are unhelpful. They are tests to avoid
regressions. I think we need to keep them.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+template requires ((sizeof(T) > 0) && ...) void f() {} //
expected-note{{previous definition is here}}
+class A;
+void operator&&(A, A);
+template requires ((sizeof(T) > 0) && ...) void f() {} //
expected-error{{redefinition of 'f'}}
ChuanqiXu9 wrote:
I still don't want to rename this as it looks not similar with the original
github issue at all...
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/ChuanqiXu9 auto_merge_enabled https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -71,9 +69,4 @@ export namespace std {
using std::operator&&;
}
-#ifdef SKIP_ODR_CHECK_IN_GMF
// expected-no-diagnostics
-#else
-// expected-error@* {{has different definitions in different modules; first
difference is defined here found data member '_S_copy_ctor' with an
initializer}}
-// expected-note@* {{but in 'a.' found data member '_S_copy_ctor' with
a different initializer}}
-#endif
mizvekov wrote:
Thanks!
This test file is also testing the same bug, I don't see anything else going on
here, so I think we could get rid of it as well.
Even then, besides:
* The naming is neither the GitHub issue nor something that describes well the
test or the problem.
* It's running multiple frontend invocations that don't make any difference.
For example, `-fskip-odr-check-in-gmf` is not a factor here, and the defines
`SKIP_ODR_CHECK_IN_GMF` and `REDUCED` are not used in the file at all.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/195983 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/195983
>From c54b8fcb7d7760929a92599a1e08d7342c24373e Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 7 Apr 2026 15:04:49 +0800
Subject: [PATCH 1/5] Reland [C++20] [Modules] Don't profiling the callee of
CXXFoldExpr (#190732)
Close https://github.com/llvm/llvm-project/issues/190333
For the test case, the root cause of the problem is, the compiler
thought the declaration of `operator &&` in consumer.cpp may change the
meaning of '&&' in the requrie clause of `F::operator()`. But it doesn't
make sense. Here we skip profiling the callee to solve the problem. Note
that we've already record the kind of the operator. So '&&' and '||'
won't be confused.
---
See the discussion in https://github.com/llvm/llvm-project/pull/194283
For the new found pattern that we may have other binary operator (e.g.,
operator +) in the require clause, e.g.,
```C++
template
requires requires(T t, U u) { t + u; }
void operator()(T, U) {}
```
This is a new problem and we need to solve it in other PR.
---
clang/lib/AST/StmtProfile.cpp | 32 -
.../callable-require-clause-merge.cppm| 35 +++
clang/test/Modules/polluted-operator.cppm | 7
.../diagnose-redefinition-fold-expr.cpp | 6
4 files changed, 72 insertions(+), 8 deletions(-)
create mode 100644 clang/test/Modules/callable-require-clause-merge.cppm
create mode 100644 clang/test/SemaCXX/diagnose-redefinition-fold-expr.cpp
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 8219e57644be6..c3bdcb9a2e60d 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ID.AddInteger(S->getOperator());
}
diff --git a/clang/test/Modules/callable-require-clause-merge.cppm
b/clang/test/Modules/callable-require-clause-merge.cppm
new file mode 100644
index 0..ae49dd7a58542
--- /dev/null
+++ b/clang/test/Modules/callable-require-clause-merge.cppm
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-reduced-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-reduced-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+
+//--- r.h
+struct F {
+ template requires ((sizeof(T) > 0) && ...)
+ void operator()(T...) {}
+} f;
+
+//--- mymod.cppm
+module;
+#include "r.h"
+export module mymod;
+export using ::f;
+
+//--- consumer.cpp
+// expected-no-diagnostics
+void operator&&(struct X, struct X);
+#include "r.h"
+import mymod;
+
+void g() { f(); }
diff --git a/clang/test/Modules/polluted-operator.cppm
b/clang/test/Modules/polluted-operator.cppm
index 45cc5e37d6a64..e81a63c3e03de 100644
--- a/clang/test/Modules/polluted-operator.cppm
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -49,8 +49,6 @@ namespace std
//--- a.cppm
module;
-// The operator&& defined in 'foo.h' will pollute the
-// expression '__is_trivial(_Types) && ...' in bar.h
#include "foo.h"
#include "bar.h"
export module a;
@@ -71,9 +69,4 @@ export namespace std {
using std::operator&&;
}
-#ifdef SKIP_ODR_CHECK_IN_GMF
// expected-no-diagnostics
-#else
-// expected-error@* {{has different definitions in different modules; first
difference is defined here
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ChuanqiXu9 wrote:
I have a different feeling. I manually reduced the issue before. The product is
clang/test/Modules/polluted-operator.cppm . I think it is precise enough to
describe the original issue. And this is the form that most people/end users
may meet in practice like the original issue reports.
And also I don't think these 2 tests are the same test. As the modules version
is about a false-positive test and the Sema test is a false-negative test. They
just have the same underlying code reason. But I don't think they are the same
test topologically.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
mizvekov wrote:
You have included the new regression test I proposed, but it's still missing:
* Clean up the comment.
* Remove the unhelpful module tests.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ChuanqiXu9 wrote:
Done. Thanks.
https://github.com/llvm/llvm-project/pull/195983
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732) (PR #195983)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/195983
>From c54b8fcb7d7760929a92599a1e08d7342c24373e Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 7 Apr 2026 15:04:49 +0800
Subject: [PATCH] Reland [C++20] [Modules] Don't profiling the callee of
CXXFoldExpr (#190732)
Close https://github.com/llvm/llvm-project/issues/190333
For the test case, the root cause of the problem is, the compiler
thought the declaration of `operator &&` in consumer.cpp may change the
meaning of '&&' in the requrie clause of `F::operator()`. But it doesn't
make sense. Here we skip profiling the callee to solve the problem. Note
that we've already record the kind of the operator. So '&&' and '||'
won't be confused.
---
See the discussion in https://github.com/llvm/llvm-project/pull/194283
For the new found pattern that we may have other binary operator (e.g.,
operator +) in the require clause, e.g.,
```C++
template
requires requires(T t, U u) { t + u; }
void operator()(T, U) {}
```
This is a new problem and we need to solve it in other PR.
---
clang/lib/AST/StmtProfile.cpp | 32 -
.../callable-require-clause-merge.cppm| 35 +++
clang/test/Modules/polluted-operator.cppm | 7
.../diagnose-redefinition-fold-expr.cpp | 6
4 files changed, 72 insertions(+), 8 deletions(-)
create mode 100644 clang/test/Modules/callable-require-clause-merge.cppm
create mode 100644 clang/test/SemaCXX/diagnose-redefinition-fold-expr.cpp
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 8219e57644be6..c3bdcb9a2e60d 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2400,7 +2400,37 @@ void StmtProfiler::VisitMaterializeTemporaryExpr(
}
void StmtProfiler::VisitCXXFoldExpr(const CXXFoldExpr *S) {
- VisitExpr(S);
+ VisitStmtNoChildren(S);
+ // We intentionally not profile the callee sub-expression
+ // to keep the profiling result stable across different
+ // context.
+ //
+ // "a.h"
+ //
+ // struct F {
+ // template requires ((sizeof(T) > 0) && ...)
+ // void operator()(T...) {}
+ // } f;
+ //
+ // and
+ //
+ // "c.h"
+ //
+ // void operator&&(struct X, struct X);
+ // #include "a.h"
+ //
+ // Here we might give different profiling results if we profile
+ // the callee sub-expression, which is nullptr in the first case
+ // an UnresolvedLookupExpr in the second case where there is a
+ // global operator&& operator that pollutes the fold expression.
+ if (S->getLHS())
+Visit(S->getLHS());
+ else
+ID.AddInteger(0);
+ if (S->getRHS())
+Visit(S->getRHS());
+ else
+ID.AddInteger(0);
ID.AddInteger(S->getOperator());
}
diff --git a/clang/test/Modules/callable-require-clause-merge.cppm
b/clang/test/Modules/callable-require-clause-merge.cppm
new file mode 100644
index 0..ae49dd7a58542
--- /dev/null
+++ b/clang/test/Modules/callable-require-clause-merge.cppm
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/mymod.cppm -emit-reduced-module-interface -o
%t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 %t/consumer.cpp -fprebuilt-module-path=%t
-fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/mymod.cppm
-emit-reduced-module-interface -o %t/mymod.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/consumer.cpp
-fprebuilt-module-path=%t -fsyntax-only -verify
+
+//--- r.h
+struct F {
+ template requires ((sizeof(T) > 0) && ...)
+ void operator()(T...) {}
+} f;
+
+//--- mymod.cppm
+module;
+#include "r.h"
+export module mymod;
+export using ::f;
+
+//--- consumer.cpp
+// expected-no-diagnostics
+void operator&&(struct X, struct X);
+#include "r.h"
+import mymod;
+
+void g() { f(); }
diff --git a/clang/test/Modules/polluted-operator.cppm
b/clang/test/Modules/polluted-operator.cppm
index 45cc5e37d6a64..e81a63c3e03de 100644
--- a/clang/test/Modules/polluted-operator.cppm
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -49,8 +49,6 @@ namespace std
//--- a.cppm
module;
-// The operator&& defined in 'foo.h' will pollute the
-// expression '__is_trivial(_Types) && ...' in bar.h
#include "foo.h"
#include "bar.h"
export module a;
@@ -71,9 +69,4 @@ export namespace std {
using std::operator&&;
}
-#ifdef SKIP_ODR_CHECK_IN_GMF
// expected-no-diagnostics
-#else
-// expected-error@* {{has different definitions in different modules; first
difference is defined here foun
