Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------

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
110 changes: 87 additions & 23 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 @@ -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;
Expand All @@ -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?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it? There is some CUDA stuff happening here, right? Otherwise why is the other condition now dead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

for (FunctionDecl *Skipped : AmbiguousDecls) {
// If skipped candidate has different CUDA preference than the result,
// there is no ambiguity. Otherwise check whether they have different
Expand Down Expand Up @@ -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). ]
Expand Down Expand Up @@ -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();
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 &); // #1
inline namespace A {
void g(short &&); // #2
}
inline namespace B {
void g(short &&); // #3
}

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