[clang] [clang] [C++20] [Modules] Don't profile UnresolvedLookupExpr in require clause and noexcept clause (PR #194283)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 26 20:44:01 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Chuanqi Xu (ChuanqiXu9)
<details>
<summary>Changes</summary>
Close https://github.com/llvm/llvm-project/issues/190333
See clang/test/Modules/callable-require-clause-merge.cppm and clang/test/Modules/polluted-operator.cppm for motivating case.
In short, the unrelated global operator may pollute the operator in require clause and noexcept clause, which makes clang emits hard to understand false-positive diagnostic message to end users.
This patch tries to avoid such problems for require clause and noexcept clause specifically.
This may not be perfect. I feel we may face other similar problems in other clause due to the design of UnresolvedLookupExpr. But on the one hand, it is better to fix it fundamentally after we saw more cases so that we can have a better understanding, on the other hand, it is better to stop blooding right now as the approach here is easy and not bad.
---
Full diff: https://github.com/llvm/llvm-project/pull/194283.diff
6 Files Affected:
- (modified) clang/include/clang/AST/Stmt.h (+26-1)
- (modified) clang/lib/AST/ASTContext.cpp (+6-2)
- (modified) clang/lib/AST/StmtProfile.cpp (+21-7)
- (modified) clang/lib/AST/Type.cpp (+9-1)
- (added) clang/test/Modules/callable-require-clause-merge.cppm (+43)
- (modified) clang/test/Modules/polluted-operator.cppm (-5)
``````````diff
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index d940aa6562c4c..5461e91ba0aa1 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1617,8 +1617,33 @@ class alignas(void *) Stmt {
/// other lambda expressions. When true, the lambda expressions with the same
/// implementation will be considered to be the same. ProfileLambdaExpr should
/// only be true when we try to merge two declarations within modules.
+ /// \param IgnoringUnresolvedLookupExpr whether or not to ignore
+ /// UnresolvedLookupExpr when profiling. When true,
+ /// IgnoringUnresolvedLookupExpr won't be invoked during profiling. This is
+ /// useful in case we don't hope the unresolved lookup expr to pollute the
+ /// profile result. e.g.,
+ ///
+ /// "a.h"
+ ///
+ /// #pragma once
+ /// 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 the `F::operator()` may produce different profiling results depending
+ /// on whether there is a freestanding `operator&&` declared before it. And
+ /// this affects declaration merging in modules.
void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
- bool Canonical, bool ProfileLambdaExpr = false) const;
+ bool Canonical, bool ProfileLambdaExpr = false,
+ bool IgnoringUnresolvedLookupExpr = false) const;
/// Calculate a unique representation for a statement that is
/// stable across compiler invocations.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index abf5f8a832043..d2298bd1b30e7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7454,8 +7454,12 @@ bool ASTContext::isSameConstraintExpr(const Expr *XCE, const Expr *YCE) const {
return true;
llvm::FoldingSetNodeID XCEID, YCEID;
- XCE->Profile(XCEID, *this, /*Canonical=*/true, /*ProfileLambdaExpr=*/true);
- YCE->Profile(YCEID, *this, /*Canonical=*/true, /*ProfileLambdaExpr=*/true);
+ /// The unresolved lookup expr may misguide the profiling results. See
+ /// clang/test/Modules/callable-require-clause-merge.cppm for an example.
+ XCE->Profile(XCEID, *this, /*Canonical=*/true, /*ProfileLambdaExpr=*/true,
+ /*IgnoringUnresolvedLookupExpr=*/true);
+ YCE->Profile(YCEID, *this, /*Canonical=*/true, /*ProfileLambdaExpr=*/true,
+ /*IgnoringUnresolvedLookupExpr=*/true);
return XCEID == YCEID;
}
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 8219e57644be6..a9c81f0aa027a 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -30,11 +30,13 @@ namespace {
llvm::FoldingSetNodeID &ID;
bool Canonical;
bool ProfileLambdaExpr;
+ bool IgnoringUnresolvedLookupExpr;
public:
StmtProfiler(llvm::FoldingSetNodeID &ID, bool Canonical,
- bool ProfileLambdaExpr)
- : ID(ID), Canonical(Canonical), ProfileLambdaExpr(ProfileLambdaExpr) {}
+ bool ProfileLambdaExpr, bool IgnoringUnresolvedLookupExpr)
+ : ID(ID), Canonical(Canonical), ProfileLambdaExpr(ProfileLambdaExpr),
+ IgnoringUnresolvedLookupExpr(IgnoringUnresolvedLookupExpr) {}
virtual ~StmtProfiler() {}
@@ -86,8 +88,11 @@ namespace {
public:
StmtProfilerWithPointers(llvm::FoldingSetNodeID &ID,
const ASTContext &Context, bool Canonical,
- bool ProfileLambdaExpr)
- : StmtProfiler(ID, Canonical, ProfileLambdaExpr), Context(Context) {}
+ bool ProfileLambdaExpr,
+ bool IgnoringUnresolvedLookupExpr)
+ : StmtProfiler(ID, Canonical, ProfileLambdaExpr,
+ IgnoringUnresolvedLookupExpr),
+ Context(Context) {}
private:
void HandleStmtClass(Stmt::StmtClass SC) override {
@@ -184,8 +189,11 @@ namespace {
class StmtProfilerWithoutPointers : public StmtProfiler {
ODRHash &Hash;
public:
+ // Set IgnoringUnresolvedLookupExpr as we don't want the unresolved lookup
+ // expr affecting the merging results.
StmtProfilerWithoutPointers(llvm::FoldingSetNodeID &ID, ODRHash &Hash)
- : StmtProfiler(ID, /*Canonical=*/false, /*ProfileLambdaExpr=*/false),
+ : StmtProfiler(ID, /*Canonical=*/false, /*ProfileLambdaExpr=*/false,
+ /*IgnoringUnresolvedLookupExpr=*/true),
Hash(Hash) {}
private:
@@ -2269,6 +2277,10 @@ void StmtProfiler::VisitOverloadExpr(const OverloadExpr *S) {
void
StmtProfiler::VisitUnresolvedLookupExpr(const UnresolvedLookupExpr *S) {
+ if (IgnoringUnresolvedLookupExpr) {
+ ID.AddInteger(0);
+ return;
+ }
VisitOverloadExpr(S);
}
@@ -2951,8 +2963,10 @@ void StmtProfiler::VisitHLSLOutArgExpr(const HLSLOutArgExpr *S) {
}
void Stmt::Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
- bool Canonical, bool ProfileLambdaExpr) const {
- StmtProfilerWithPointers Profiler(ID, Context, Canonical, ProfileLambdaExpr);
+ bool Canonical, bool ProfileLambdaExpr,
+ bool IgnoringUnresolvedLookupExpr) const {
+ StmtProfilerWithPointers Profiler(ID, Context, Canonical, ProfileLambdaExpr,
+ IgnoringUnresolvedLookupExpr);
Profiler.Visit(this);
}
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 6c295c1a9c409..87ab7cd3ad185 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4035,7 +4035,15 @@ void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID, QualType Result,
for (QualType Ex : epi.ExceptionSpec.Exceptions)
ID.AddPointer(Ex.getAsOpaquePtr());
} else if (isComputedNoexcept(epi.ExceptionSpec.Type)) {
- epi.ExceptionSpec.NoexceptExpr->Profile(ID, Context, Canonical);
+ // Make sure the profiling result of the noexcept expression
+ // won't be affected by the unresolved lookup expressions.
+ // See clang/test/Modules/polluted-operator.cppm for an example
+ // for it.
+ //
+ // ProfileLambdaExpr=false is the default value.
+ epi.ExceptionSpec.NoexceptExpr->Profile(
+ ID, Context, Canonical, /*ProfileLambdaExpr=*/false,
+ /*IgnoringUnresolvedLookupExpr=*/true);
} else if (epi.ExceptionSpec.Type == EST_Uninstantiated ||
epi.ExceptionSpec.Type == EST_Unevaluated) {
ID.AddPointer(epi.ExceptionSpec.SourceDecl->getCanonicalDecl());
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..6cdb369639a39
--- /dev/null
+++ b/clang/test/Modules/callable-require-clause-merge.cppm
@@ -0,0 +1,43 @@
+// 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;
+
+struct G {
+ template <typename T, typename U>
+ requires requires(T t, U u) { t + u; }
+ void operator()(T, U) {}
+} g;
+
+//--- mymod.cppm
+module;
+#include "r.h"
+export module mymod;
+export using ::f;
+export using ::g;
+
+//--- consumer.cpp
+// expected-no-diagnostics
+void operator&&(struct X, struct X);
+void operator+(struct X, struct Y);
+#include "r.h"
+import mymod;
+
+void h() { f(); g(1, 2); }
diff --git a/clang/test/Modules/polluted-operator.cppm b/clang/test/Modules/polluted-operator.cppm
index 45cc5e37d6a64..e6f0cdf092414 100644
--- a/clang/test/Modules/polluted-operator.cppm
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -71,9 +71,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
``````````
</details>
https://github.com/llvm/llvm-project/pull/194283
More information about the cfe-commits
mailing list