Reland [C++20] [Modules] Don't profiling the callee of CXXFoldExpr (#190732)#195983
Merged
Conversation
llvmorg-github-actions
Bot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) ChangesClose #190333 For the test case, the root cause of the problem is, the compiler thought the declaration of See the discussion in #194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., template <typename T, typename U>
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. Full diff: https://github.com/llvm/llvm-project/pull/195983.diff 3 Files Affected:
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 <typename... T> 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 0000000000000..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 <typename... T> 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 found data member '_S_copy_ctor' with an initializer}}
-// expected-note@* {{but in 'a.<global>' found data member '_S_copy_ctor' with a different initializer}}
-#endif
|
mizvekov
reviewed
May 6, 2026
…lvm#190732) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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.
ChuanqiXu9
force-pushed
the
DontProfileCXXFoldExprV2
branch
from
dafa586 to
c54b8fc
Compare
mizvekov
reviewed
May 8, 2026
mizvekov
reviewed
May 8, 2026
mizvekov
approved these changes
May 8, 2026
Member
Author
|
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 |
moar55
pushed a commit
to moar55/llvm-project
that referenced
this pull request
May 12, 2026
…lvm#190732) (llvm#195983) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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.
EuphoricThinking
pushed a commit
to EuphoricThinking/llvm-project
that referenced
this pull request
May 14, 2026
…lvm#190732) (llvm#195983) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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.
pedroMVicente
pushed a commit
to pedroMVicente/llvm-project
that referenced
this pull request
May 19, 2026
…lvm#190732) (llvm#195983) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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.
c-rhodes
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
May 26, 2026
…lvm#190732) (llvm#195983) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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. (cherry picked from commit 2751c7e)
daunabomba
pushed a commit
to daunabomba/llvm-project
that referenced
this pull request
Jun 2, 2026
…lvm#190732) (llvm#195983) Close llvm#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 llvm#194283 For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g., ```C++ template <typename T, typename U> 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. (cherry picked from commit 2751c7e)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #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 ofF::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 #194283
For the new found pattern that we may have other binary operator (e.g., operator +) in the require clause, e.g.,
This is a new problem and we need to solve it in other PR.