Skip to content

[Clang] Implement CWG2918 'Consideration of constraints for address of overloaded function' #127773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ 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
------------------

Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1773,20 +1773,19 @@ 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();
return Kind == FunctionDecl::TK_NonTemplate ||
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();
Expand Down
115 changes: 89 additions & 26 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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);
Comment on lines +13285 to +13300
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

void EliminateSuboptimalCudaMatches() {
S.CUDA().EraseUnwantedMatches(S.getCurFunctionDecl(/*AllowLambda=*/true),
Matches);
Expand Down Expand Up @@ -13408,8 +13456,8 @@ Sema::resolveAddressOfSingleOverloadCandidate(Expr *E, DeclAccessPair &Pair) {
Result = FD;
};

// We have more than one result - see if it is more constrained than the
// previous one.
// We have more than one result - see if it is more
// partial-ordering-constrained than the previous one.
if (Result) {
// Check CUDA preference first. If the candidates have differennt CUDA
// preference, choose the one with higher CUDA preference. Otherwise,
Expand All @@ -13424,9 +13472,17 @@ Sema::resolveAddressOfSingleOverloadCandidate(Expr *E, DeclAccessPair &Pair) {
continue;
}
}
// FD has the same CUDA prefernece than Result. Continue check
// FD has the same CUDA preference than Result. Continue to 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;
Expand All @@ -13443,7 +13499,6 @@ 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.
for (FunctionDecl *Skipped : AmbiguousDecls) {
Expand Down Expand Up @@ -13489,7 +13544,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). ]
Expand Down Expand Up @@ -13540,8 +13595,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();
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/Sema/SemaTemplateDeduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
86 changes: 86 additions & 0 deletions clang/test/CXX/drs/cwg29xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,92 @@ 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(y), 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
#endif

#if __cplusplus >= 201103L
namespace Example3 {

template <typename T> void f(T &&, void (*)(T &&)); // #cwg2918_f

void g(int &);
inline namespace A {
void g(short &&);
}
inline namespace B {
void g(short &&);
}

void q() {
int x;
f(x, g);
// since-cxx11-error@-1 {{no matching function for call to 'f'}}
// since-cxx11-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));
Expand Down
2 changes: 1 addition & 1 deletion clang/www/cxx_dr_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down