[Clang] Implement CWG2918 'Consideration of constraints for address of overloaded function' #127773
Conversation
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
…f overloaded function'
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesCloses #122523 Full diff: https://github.com/llvm/llvm-project/pull/127773.diff 7 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a91c764860ccd..b876b48c63717 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -88,6 +88,10 @@ Resolutions to C++ Defect Reports
two releases. The improvements to template template parameter matching implemented
in the previous release, as described in P3310 and P3579, made this flag unnecessary.
+- Implemented `CWG2918 Consideration of constraints for address of overloaded `
+ `function <https://cplusplus.github.io/CWG/issues/2918.html>`_
+
+
C Language Changes
------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c55b964650323..3270e7afb3796 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10466,7 +10466,8 @@ class Sema final : public SemaBase {
/// returned.
FunctionDecl *ResolveSingleFunctionTemplateSpecialization(
OverloadExpr *ovl, bool Complain = false, DeclAccessPair *Found = nullptr,
- TemplateSpecCandidateSet *FailedTSC = nullptr);
+ TemplateSpecCandidateSet *FailedTSC = nullptr,
+ bool ForTypeDeduction = false);
// Resolve and fix an overloaded expression that can be resolved
// because it identifies a single function template specialization.
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 8a77cbf8c9477..f8bc176c1d8b5 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1773,6 +1773,7 @@ bool Sema::IsAtLeastAsConstrained(NamedDecl *D1,
NamedDecl *D2,
MutableArrayRef<const Expr *> AC2,
bool &Result) {
+#ifndef NDEBUG
if (const auto *FD1 = dyn_cast<FunctionDecl>(D1)) {
auto IsExpectedEntity = [](const FunctionDecl *FD) {
FunctionDecl::TemplatedKind Kind = FD->getTemplatedKind();
@@ -1780,13 +1781,11 @@ bool Sema::IsAtLeastAsConstrained(NamedDecl *D1,
Kind == FunctionDecl::TK_FunctionTemplate;
};
const auto *FD2 = dyn_cast<FunctionDecl>(D2);
- (void)IsExpectedEntity;
- (void)FD1;
- (void)FD2;
assert(IsExpectedEntity(FD1) && FD2 && IsExpectedEntity(FD2) &&
"use non-instantiated function declaration for constraints partial "
"ordering");
}
+#endif
if (AC1.empty()) {
Result = AC2.empty();
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 8d5b5ac190b5b..77fc78dd8ce99 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10369,20 +10369,16 @@ static bool allowAmbiguity(ASTContext &Context, const FunctionDecl *F1,
/// [over.match.best.general]p2.6
/// F1 and F2 are non-template functions with the same
/// non-object-parameter-type-lists, and F1 is more constrained than F2 [...]
-static bool sameFunctionParameterTypeLists(Sema &S,
- const OverloadCandidate &Cand1,
- const OverloadCandidate &Cand2) {
- if (!Cand1.Function || !Cand2.Function)
- return false;
-
- FunctionDecl *Fn1 = Cand1.Function;
- FunctionDecl *Fn2 = Cand2.Function;
-
+static bool sameFunctionParameterTypeLists(Sema &S, FunctionDecl *Fn1,
+ FunctionDecl *Fn2,
+ bool IsFn1Reversed,
+ bool IsFn2Reversed) {
+ assert(Fn1 && Fn2);
if (Fn1->isVariadic() != Fn2->isVariadic())
return false;
- if (!S.FunctionNonObjectParamTypesAreEqual(
- Fn1, Fn2, nullptr, Cand1.isReversed() ^ Cand2.isReversed()))
+ if (!S.FunctionNonObjectParamTypesAreEqual(Fn1, Fn2, nullptr,
+ IsFn1Reversed ^ IsFn2Reversed))
return false;
auto *Mem1 = dyn_cast<CXXMethodDecl>(Fn1);
@@ -10403,6 +10399,30 @@ static bool sameFunctionParameterTypeLists(Sema &S,
return true;
}
+static FunctionDecl *
+getMorePartialOrderingConstrained(Sema &S, FunctionDecl *Fn1, FunctionDecl *Fn2,
+ bool IsFn1Reversed, bool IsFn2Reversed) {
+ if (!Fn1 || !Fn2)
+ return nullptr;
+
+ // C++ [temp.constr.order]:
+ // A non-template function F1 is more partial-ordering-constrained than a
+ // non-template function F2 if:
+ bool Cand1IsSpecialization = Fn1->getPrimaryTemplate();
+ bool Cand2IsSpecialization = Fn2->getPrimaryTemplate();
+
+ if (Cand1IsSpecialization || Cand2IsSpecialization)
+ return nullptr;
+
+ // - they have the same non-object-parameter-type-lists, and [...]
+ if (!sameFunctionParameterTypeLists(S, Fn1, Fn2, IsFn1Reversed,
+ IsFn2Reversed))
+ return nullptr;
+
+ // - the declaration of F1 is more constrained than the declaration of F2.
+ return S.getMoreConstrainedFunction(Fn1, Fn2);
+}
+
/// isBetterOverloadCandidate - Determines whether the first overload
/// candidate is a better candidate than the second (C++ 13.3.3p1).
bool clang::isBetterOverloadCandidate(
@@ -10649,12 +10669,12 @@ bool clang::isBetterOverloadCandidate(
}
}
- // -— F1 and F2 are non-template functions with the same
- // parameter-type-lists, and F1 is more constrained than F2 [...],
- if (!Cand1IsSpecialization && !Cand2IsSpecialization &&
- sameFunctionParameterTypeLists(S, Cand1, Cand2) &&
- S.getMoreConstrainedFunction(Cand1.Function, Cand2.Function) ==
- Cand1.Function)
+ // -— F1 and F2 are non-template functions and F1 is more
+ // partial-ordering-constrained than F2 [...],
+ if (FunctionDecl *F = getMorePartialOrderingConstrained(
+ S, Cand1.Function, Cand2.Function, Cand1.isReversed(),
+ Cand2.isReversed());
+ F && F == Cand1.Function)
return true;
// -- F1 is a constructor for a class D, F2 is a constructor for a base
@@ -12999,9 +13019,10 @@ class AddressOfFunctionResolver {
// C++ [over.over]p4:
// If more than one function is selected, [...]
if (Matches.size() > 1 && !eliminiateSuboptimalOverloadCandidates()) {
- if (FoundNonTemplateFunction)
+ if (FoundNonTemplateFunction) {
EliminateAllTemplateMatches();
- else
+ EliminateLessPartialOrderingConstrainedMatches();
+ } else
EliminateAllExceptMostSpecializedTemplate();
}
}
@@ -13252,6 +13273,33 @@ class AddressOfFunctionResolver {
}
}
+ void EliminateLessPartialOrderingConstrainedMatches() {
+ // C++ [over.over]p5:
+ // [...] Any given non-template function F0 is eliminated if the set
+ // contains a second non-template function that is more
+ // partial-ordering-constrained than F0. [...]
+ assert(Matches[0].second->getPrimaryTemplate() == nullptr &&
+ "Call EliminateAllTemplateMatches() first");
+ SmallVector<std::pair<DeclAccessPair, FunctionDecl *>, 4> Results;
+ Results.push_back(Matches[0]);
+ for (unsigned I = 1, N = Matches.size(); I < N; ++I) {
+ assert(Matches[I].second->getPrimaryTemplate() == nullptr);
+ FunctionDecl *F = getMorePartialOrderingConstrained(
+ S, Matches[I].second, Results[0].second,
+ /*IsFn1Reversed=*/false,
+ /*IsFn2Reversed=*/false);
+ if (!F) {
+ Results.push_back(Matches[I]);
+ continue;
+ }
+ if (F == Matches[I].second) {
+ Results.clear();
+ Results.push_back(Matches[I]);
+ }
+ }
+ std::swap(Matches, Results);
+ }
+
void EliminateSuboptimalCudaMatches() {
S.CUDA().EraseUnwantedMatches(S.getCurFunctionDecl(/*AllowLambda=*/true),
Matches);
@@ -13426,7 +13474,15 @@ Sema::resolveAddressOfSingleOverloadCandidate(Expr *E, DeclAccessPair &Pair) {
}
// FD has the same CUDA prefernece than Result. Continue check
// constraints.
- FunctionDecl *MoreConstrained = getMoreConstrainedFunction(FD, Result);
+
+ // C++ [over.over]p5:
+ // [...] Any given non-template function F0 is eliminated if the set
+ // contains a second non-template function that is more
+ // partial-ordering-constrained than F0 [...]
+ FunctionDecl *MoreConstrained =
+ getMorePartialOrderingConstrained(*this, FD, Result,
+ /*IsFn1Reversed=*/false,
+ /*IsFn2Reversed=*/false);
if (MoreConstrained != FD) {
if (!MoreConstrained) {
IsResultAmbiguous = true;
@@ -13443,9 +13499,9 @@ Sema::resolveAddressOfSingleOverloadCandidate(Expr *E, DeclAccessPair &Pair) {
return nullptr;
if (Result) {
- SmallVector<const Expr *, 1> ResultAC;
// We skipped over some ambiguous declarations which might be ambiguous with
// the selected result.
+ // FIXME: This for-loop is dead code. Remove it?
for (FunctionDecl *Skipped : AmbiguousDecls) {
// If skipped candidate has different CUDA preference than the result,
// there is no ambiguity. Otherwise check whether they have different
@@ -13489,7 +13545,7 @@ bool Sema::resolveAndFixAddressOfSingleOverloadCandidate(
FunctionDecl *Sema::ResolveSingleFunctionTemplateSpecialization(
OverloadExpr *ovl, bool Complain, DeclAccessPair *FoundResult,
- TemplateSpecCandidateSet *FailedTSC) {
+ TemplateSpecCandidateSet *FailedTSC, bool ForTypeDeduction) {
// C++ [over.over]p1:
// [...] [Note: any redundant set of parentheses surrounding the
// overloaded function name is ignored (5.1). ]
@@ -13540,8 +13596,16 @@ FunctionDecl *Sema::ResolveSingleFunctionTemplateSpecialization(
assert(Specialization && "no specialization and no error?");
- // Multiple matches; we can't resolve to a single declaration.
+ // C++ [temp.deduct.call]p6:
+ // [...] If all successful deductions yield the same deduced A, that
+ // deduced A is the result of deduction; otherwise, the parameter is
+ // treated as a non-deduced context.
if (Matched) {
+ if (ForTypeDeduction &&
+ isSameOrCompatibleFunctionType(Matched->getType(),
+ Specialization->getType()))
+ continue;
+ // Multiple matches; we can't resolve to a single declaration.
if (Complain) {
Diag(ovl->getExprLoc(), diag::err_addr_ovl_ambiguous)
<< ovl->getName();
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index eb70dd743c8be..627cd82ed1c77 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -4252,7 +4252,8 @@ ResolveOverloadForDeduction(Sema &S, TemplateParameterList *TemplateParams,
if (FunctionDecl *ExplicitSpec =
S.ResolveSingleFunctionTemplateSpecialization(
Ovl, /*Complain=*/false,
- /*FoundDeclAccessPair=*/nullptr, FailedTSC))
+ /*Found=*/nullptr, FailedTSC,
+ /*ForTypeDeduction=*/true))
return GetTypeOfFunction(S, R, ExplicitSpec);
}
@@ -4321,7 +4322,11 @@ ResolveOverloadForDeduction(Sema &S, TemplateParameterList *TemplateParams,
/*HasDeducedAnyParam=*/nullptr);
if (Result != TemplateDeductionResult::Success)
continue;
- if (!Match.isNull())
+ // C++ [temp.deduct.call]p6:
+ // [...] If all successful deductions yield the same deduced A, that
+ // deduced A is the result of deduction; otherwise, the parameter is
+ // treated as a non-deduced context. [...]
+ if (!Match.isNull() && !S.isSameOrCompatibleFunctionType(Match, ArgType))
return {};
Match = ArgType;
}
diff --git a/clang/test/CXX/drs/cwg29xx.cpp b/clang/test/CXX/drs/cwg29xx.cpp
index aeb62b8d68710..61c3ecc04b895 100644
--- a/clang/test/CXX/drs/cwg29xx.cpp
+++ b/clang/test/CXX/drs/cwg29xx.cpp
@@ -60,6 +60,90 @@ struct S {
#endif
} // namespace cwg2917
+namespace cwg2918 { // cwg2918: 21
+
+#if __cplusplus >= 202002L
+
+namespace Example1 {
+
+template<bool B> struct X {
+ void f(short) requires B;
+ void f(long);
+ template<typename> void g(short) requires B;
+ template<typename> void g(long);
+};
+
+void test() {
+ &X<true>::f; // since-cxx20-error {{reference to overloaded function could not be resolved}}
+ &X<true>::g<int>; // since-cxx20-error {{reference to overloaded function could not be resolved}}
+}
+
+} // namespace Example1
+
+namespace Example2 {
+
+template <bool B> struct X {
+ static constexpr int f(short) requires B {
+ return 42;
+ }
+ static constexpr int f(short) {
+ return 24;
+ }
+};
+
+template <typename T>
+constexpr int f(T) { return 1; }
+
+template <typename T>
+ requires __is_same(T, int)
+constexpr int f(T) { return 2; }
+
+void test() {
+ constexpr auto x = &X<true>::f;
+ static_assert(__is_same(decltype(x), int(*const)(short)), "");
+ static_assert(x(0) == 42, "");
+
+ constexpr auto y = &X<false>::f;
+ static_assert(__is_same(decltype(x), int(*const)(short)));
+ static_assert(y(0) == 24, "");
+
+ constexpr auto z = &f<int>;
+ static_assert(__is_same(decltype(z), int(*const)(int)));
+ static_assert(z(0) == 2, "");
+
+ // C++ [temp.deduct.call]p6:
+ // If the argument is an overload set containing one or more function templates,
+ // the parameter is treated as a non-deduced context.
+ auto w = f; // since-cxx20-error {{variable 'w' with type 'auto' has incompatible initializer of type '<overloaded function type>'}}
+}
+
+} // namespace Example2
+
+namespace Example3 {
+
+template <typename T> void f(T &&, void (*)(T &&)); // #cwg2918_f
+
+void g(int &); // #1
+inline namespace A {
+void g(short &&); // #2
+}
+inline namespace B {
+void g(short &&); // #3
+}
+
+void q() {
+ int x;
+ f(x, g);
+ // since-cxx20-error@-1 {{no matching function for call to 'f'}}
+ // since-cxx20-note@#cwg2918_f {{candidate template ignored: deduced conflicting types for parameter 'T' ('int &' vs. 'short')}}
+}
+
+} // namespace Example3
+
+#endif
+
+} // namespace cwg2918
+
#if __cplusplus > 202302L
namespace std {
using size_t = decltype(sizeof(0));
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 69ddd5e58b921..b674f8f094bb8 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -17364,7 +17364,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2918.html">2918</a></td>
<td>DR</td>
<td>Consideration of constraints for address of overloaded function</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 21</td>
</tr>
<tr id="2919">
<td><a href="https://cplusplus.github.io/CWG/issues/2919.html">2919</a></td>
|
Endilll
reviewed
Feb 19, 2025
erichkeane
reviewed
Feb 19, 2025
clang/lib/Sema/SemaOverload.cpp
Outdated
| // We skipped over some ambiguous declarations which might be ambiguous with | ||
| // the selected result. | ||
| // FIXME: This for-loop is dead code. Remove it? |
There was a problem hiding this comment.
Is it? There is some CUDA stuff happening here, right? Otherwise why is the other condition now dead?
There was a problem hiding this comment.
The only way AmbiguousDecls comes with elements is on line 13433, where we also set IsResultAmbiguous to true. However, we bail out of the function on line 13442 when IsResultAmbiguous
There was a problem hiding this comment.
Hmm... perhaps this was supposed to be higher? in the code? I'm concerned about it, but if no tests fail, then I think we have to go with it. Just kill the loop.
mizvekov
reviewed
Feb 19, 2025
Comment on lines
+13285
to
+13300
| for (unsigned I = 1, N = Matches.size(); I < N; ++I) { | ||
| assert(Matches[I].second->getPrimaryTemplate() == nullptr); | ||
| FunctionDecl *F = getMorePartialOrderingConstrained( | ||
| S, Matches[I].second, Results[0].second, | ||
| /*IsFn1Reversed=*/false, | ||
| /*IsFn2Reversed=*/false); | ||
| if (!F) { | ||
| Results.push_back(Matches[I]); | ||
| continue; | ||
| } | ||
| if (F == Matches[I].second) { | ||
| Results.clear(); | ||
| Results.push_back(Matches[I]); | ||
| } | ||
| } | ||
| std::swap(Matches, Results); |
There was a problem hiding this comment.
Does this work if there are pairs of functions in the set which are not more constrained than the other both ways?
Do we have existing tests which cover that?
There was a problem hiding this comment.
It is supposed to work, though I haven't yet devised a test to exercise that branch. My understanding is that if there were more than one candidate that isn't more constrained than the others, we should have ruled them out earlier when checking the full address-taken expression, namely in resolveAddressOfSingleOverloadCandidate(), whose logic is similar to this AddressOfFunctionResolver::EliminateLessPartialOrderingConstrainedMatches, which is mainly used for initialization stuff purpose.
We unfortunately ended up with these separate pieces of address-of resolution logic for different scenarios such as type deduction and initialization sequence construction. While we might be doing some redundant work, from my understanding, the function in its current form aligns with the standard's intent.
Endilll
reviewed
Feb 20, 2025
erichkeane
approved these changes
Feb 21, 2025
clang/lib/Sema/SemaOverload.cpp
Outdated
| // We skipped over some ambiguous declarations which might be ambiguous with | ||
| // the selected result. | ||
| // FIXME: This for-loop is dead code. Remove it? |
There was a problem hiding this comment.
Hmm... perhaps this was supposed to be higher? in the code? I'm concerned about it, but if no tests fail, then I think we have to go with it. Just kill the loop.
|
I'll merge it as-is now - post review comments are welcome |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #122523