From fd7960f1c9d61f45e97d5a65b524b00c129427a8 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Mon, 27 Apr 2026 11:33:37 +0800 Subject: [PATCH 1/2] [clang] [C++20] [Modules] Don't profile UnresolvedLookupExpr in require clause and noexcept clause 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 and have more insights, on the other hand, it is better to stop blooding right now as the approach here is easy and not bad. --- clang/include/clang/AST/Stmt.h | 27 +++++++++++- clang/lib/AST/ASTContext.cpp | 8 +++- clang/lib/AST/StmtProfile.cpp | 28 +++++++++--- clang/lib/AST/Type.cpp | 10 ++++- .../callable-require-clause-merge.cppm | 43 +++++++++++++++++++ clang/test/Modules/polluted-operator.cppm | 5 --- 6 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 clang/test/Modules/callable-require-clause-merge.cppm 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 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 requires ((sizeof(T) > 0) && ...) + void operator()(T...) {} +} f; + +struct G { + template + 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.' found data member '_S_copy_ctor' with a different initializer}} -#endif From 744b4241a09ed2fd46186a832823f3743d50bd12 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 28 Apr 2026 11:15:35 +0800 Subject: [PATCH 2/2] Only ignoring UnresolvedLookupExpr during comparing entity during merging --- clang/include/clang/AST/ASTContext.h | 12 ++++++++--- clang/include/clang/AST/TypeBase.h | 3 ++- clang/lib/AST/ASTContext.cpp | 30 ++++++++++++++++++---------- clang/lib/AST/Type.cpp | 12 +++++------ 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index c45d54fdd2e88..b2be1d0a1e70b 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1854,9 +1854,15 @@ class ASTContext : public RefCountedBase { private: /// Return a normal function type with a typed argument list. - QualType getFunctionTypeInternal(QualType ResultTy, ArrayRef Args, - const FunctionProtoType::ExtProtoInfo &EPI, - bool OnlyWantCanonical) const; + QualType + getFunctionTypeInternal(QualType ResultTy, ArrayRef Args, + const FunctionProtoType::ExtProtoInfo &EPI, + bool OnlyWantCanonical, + bool IgnoringUnresolvedLookupExpr = false) const; + + QualType getFunctionTypeWithExceptionSpecInternal( + QualType Orig, const FunctionProtoType::ExceptionSpecInfo &ESI, + bool IgnoringUnresolvedLookupExpr) const; public: QualType getTypeDeclType(ElaboratedTypeKeyword Keyword, diff --git a/clang/include/clang/AST/TypeBase.h b/clang/include/clang/AST/TypeBase.h index b2887bcc36246..b0b2ff18e7fd8 100644 --- a/clang/include/clang/AST/TypeBase.h +++ b/clang/include/clang/AST/TypeBase.h @@ -5953,7 +5953,8 @@ class FunctionProtoType final static void Profile(llvm::FoldingSetNodeID &ID, QualType Result, param_type_iterator ArgTys, unsigned NumArgs, const ExtProtoInfo &EPI, const ASTContext &Context, - bool Canonical); + bool Canonical, + bool IgnoringUnresolvedLookupExpr = false); }; /// The elaboration keyword that precedes a qualified type name or diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index d2298bd1b30e7..2f3ac9dc91529 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -3794,21 +3794,32 @@ void ASTContext::adjustDeducedFunctionResultType(FunctionDecl *FD, /// specified exception specification. Type sugar that can be present on a /// declaration of a function with an exception specification is permitted /// and preserved. Other type sugar (for instance, typedefs) is not. -QualType ASTContext::getFunctionTypeWithExceptionSpec( - QualType Orig, const FunctionProtoType::ExceptionSpecInfo &ESI) const { +QualType ASTContext::getFunctionTypeWithExceptionSpecInternal( + QualType Orig, const FunctionProtoType::ExceptionSpecInfo &ESI, + bool IgnoringUnresolvedLookupExpr) const { return adjustType(Orig, [&](QualType Ty) { const auto *Proto = Ty->castAs(); - return getFunctionType(Proto->getReturnType(), Proto->getParamTypes(), - Proto->getExtProtoInfo().withExceptionSpec(ESI)); + return getFunctionTypeInternal( + Proto->getReturnType(), Proto->getParamTypes(), + Proto->getExtProtoInfo().withExceptionSpec(ESI), + /*OnlyWantCanonical=*/false, IgnoringUnresolvedLookupExpr); }); } +QualType ASTContext::getFunctionTypeWithExceptionSpec( + QualType Orig, const FunctionProtoType::ExceptionSpecInfo &ESI) const { + return getFunctionTypeWithExceptionSpecInternal( + Orig, ESI, /*IgnoringUnresolvedLookupExpr=*/false); +} + bool ASTContext::hasSameFunctionTypeIgnoringExceptionSpec(QualType T, QualType U) const { return hasSameType(T, U) || (getLangOpts().CPlusPlus17 && - hasSameType(getFunctionTypeWithExceptionSpec(T, EST_None), - getFunctionTypeWithExceptionSpec(U, EST_None))); + hasSameType(getFunctionTypeWithExceptionSpecInternal( + T, EST_None, /*IgnoringUnresolvedLookupExpr=*/true), + getFunctionTypeWithExceptionSpecInternal( + U, EST_None, /*IgnoringUnresolvedLookupExpr=*/true))); } QualType ASTContext::getFunctionTypeWithoutPtrSizes(QualType T) { @@ -4988,14 +4999,15 @@ static bool isCanonicalExceptionSpecification( QualType ASTContext::getFunctionTypeInternal( QualType ResultTy, ArrayRef ArgArray, - const FunctionProtoType::ExtProtoInfo &EPI, bool OnlyWantCanonical) const { + const FunctionProtoType::ExtProtoInfo &EPI, bool OnlyWantCanonical, + bool IgnoringUnresolvedLookupExpr) const { size_t NumArgs = ArgArray.size(); // Unique functions, to guarantee there is only one function of a particular // structure. llvm::FoldingSetNodeID ID; FunctionProtoType::Profile(ID, ResultTy, ArgArray.begin(), NumArgs, EPI, - *this, true); + *this, true, IgnoringUnresolvedLookupExpr); QualType Canonical; bool Unique = false; @@ -7783,8 +7795,6 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const { auto *XFPT = XT->getAs(); auto *YFPT = YT->getAs(); if (getLangOpts().CPlusPlus17 && XFPT && YFPT && - (isUnresolvedExceptionSpec(XFPT->getExceptionSpecType()) || - isUnresolvedExceptionSpec(YFPT->getExceptionSpecType())) && hasSameFunctionTypeIgnoringExceptionSpec(XT, YT)) return true; return false; diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index 87ab7cd3ad185..1675dc1d6a560 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -3996,7 +3996,8 @@ bool FunctionProtoType::isTemplateVariadic() const { void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID, QualType Result, const QualType *ArgTys, unsigned NumParams, const ExtProtoInfo &epi, - const ASTContext &Context, bool Canonical) { + const ASTContext &Context, bool Canonical, + bool IgnoringUnresolvedLookupExpr) { // We have to be careful not to get ambiguous profile encodings. // Note that valid type pointers are never ambiguous with anything else. // @@ -4035,15 +4036,14 @@ void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID, QualType Result, for (QualType Ex : epi.ExceptionSpec.Exceptions) ID.AddPointer(Ex.getAsOpaquePtr()); } else if (isComputedNoexcept(epi.ExceptionSpec.Type)) { - // Make sure the profiling result of the noexcept expression - // won't be affected by the unresolved lookup expressions. + // It may be problematic if we don't ignore the unresolved lookup expr. // 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); + epi.ExceptionSpec.NoexceptExpr->Profile(ID, Context, Canonical, + /*ProfileLambdaExpr=*/false, + IgnoringUnresolvedLookupExpr); } else if (epi.ExceptionSpec.Type == EST_Uninstantiated || epi.ExceptionSpec.Type == EST_Unevaluated) { ID.AddPointer(epi.ExceptionSpec.SourceDecl->getCanonicalDecl());