Skip to content

Commit b997ff4

Browse files
authored
Revert [clang] Handle templated operators with reversed arguments and [STLExtras] Undo C++20 hack (llvm#69937)
This breaks C++20 build of LLVM by clang 17 and earlier. Next steps should be reduce error to a warning for https://godbolt.org/z/s99bvq4sG b100ca6 or similar should be reapplied after the bug fix reached clang-18.
1 parent 7030623 commit b997ff4

File tree

3 files changed

+19
-44
lines changed

3 files changed

+19
-44
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,27 +37,6 @@ These changes are ones which we think may surprise users when upgrading to
3737
Clang |release| because of the opportunity they pose for disruption to existing
3838
code bases.
3939

40-
- Fix a bug in reversed argument for templated operators.
41-
This breaks code in C++20 which was previously accepted in C++17. Eg:
42-
43-
.. code-block:: cpp
44-
45-
struct P {};
46-
template<class S> bool operator==(const P&, const S&);
47-
48-
struct A : public P {};
49-
struct B : public P {};
50-
51-
// This equality is now ambiguous in C++20.
52-
bool ambiguous(A a, B b) { return a == b; }
53-
54-
template<class S> bool operator!=(const P&, const S&);
55-
// Ok. Found a matching operator!=.
56-
bool fine(A a, B b) { return a == b; }
57-
58-
To reduce such widespread breakages, as an extension, Clang accepts this code
59-
with an existing warning ``-Wambiguous-reversed-operator`` warning.
60-
Fixes `GH <https://github.com/llvm/llvm-project/issues/53954>`_.
6140

6241
C/C++ Language Potentially Breaking Changes
6342
-------------------------------------------

clang/lib/Sema/SemaOverload.cpp

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7685,7 +7685,7 @@ bool Sema::CheckNonDependentConversions(
76857685
QualType ParamType = ParamTypes[I + Offset];
76867686
if (!ParamType->isDependentType()) {
76877687
unsigned ConvIdx = PO == OverloadCandidateParamOrder::Reversed
7688-
? Args.size() - 1 - (ThisConversions + I)
7688+
? 0
76897689
: (ThisConversions + I);
76907690
Conversions[ConvIdx]
76917691
= TryCopyInitialization(*this, Args[I], ParamType,
@@ -10082,19 +10082,11 @@ getImplicitObjectParamType(ASTContext &Context, const FunctionDecl *F) {
1008210082
return M->getFunctionObjectParameterReferenceType();
1008310083
}
1008410084

10085-
// As a Clang extension, allow ambiguity among F1 and F2 if they represent
10086-
// represent the same entity.
10087-
static bool allowAmbiguity(ASTContext &Context, const FunctionDecl *F1,
10088-
const FunctionDecl *F2) {
10085+
static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
10086+
const FunctionDecl *F2) {
1008910087
if (declaresSameEntity(F1, F2))
1009010088
return true;
10091-
if (F1->isTemplateInstantiation() && F2->isTemplateInstantiation() &&
10092-
declaresSameEntity(F1->getPrimaryTemplate(), F2->getPrimaryTemplate())) {
10093-
return true;
10094-
}
10095-
// TODO: It is not clear whether comparing parameters is necessary (i.e.
10096-
// different functions with same params). Consider removing this (as no test
10097-
// fail w/o it).
10089+
1009810090
auto NextParam = [&](const FunctionDecl *F, unsigned &I, bool First) {
1009910091
if (First) {
1010010092
if (std::optional<QualType> T = getImplicitObjectParamType(Context, F))
@@ -10279,14 +10271,14 @@ bool clang::isBetterOverloadCandidate(
1027910271
case ImplicitConversionSequence::Worse:
1028010272
if (Cand1.Function && Cand2.Function &&
1028110273
Cand1.isReversed() != Cand2.isReversed() &&
10282-
allowAmbiguity(S.Context, Cand1.Function, Cand2.Function)) {
10274+
haveSameParameterTypes(S.Context, Cand1.Function, Cand2.Function)) {
1028310275
// Work around large-scale breakage caused by considering reversed
1028410276
// forms of operator== in C++20:
1028510277
//
10286-
// When comparing a function against a reversed function, if we have a
10287-
// better conversion for one argument and a worse conversion for the
10288-
// other, the implicit conversion sequences are treated as being equally
10289-
// good.
10278+
// When comparing a function against a reversed function with the same
10279+
// parameter types, if we have a better conversion for one argument and
10280+
// a worse conversion for the other, the implicit conversion sequences
10281+
// are treated as being equally good.
1029010282
//
1029110283
// This prevents a comparison function from being considered ambiguous
1029210284
// with a reversed form that is written in the same way.
@@ -14457,7 +14449,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
1445714449
llvm::SmallVector<FunctionDecl*, 4> AmbiguousWith;
1445814450
for (OverloadCandidate &Cand : CandidateSet) {
1445914451
if (Cand.Viable && Cand.Function && Cand.isReversed() &&
14460-
allowAmbiguity(Context, Cand.Function, FnDecl)) {
14452+
haveSameParameterTypes(Context, Cand.Function, FnDecl)) {
1446114453
for (unsigned ArgIdx = 0; ArgIdx < 2; ++ArgIdx) {
1446214454
if (CompareImplicitConversionSequences(
1446314455
*this, OpLoc, Cand.Conversions[ArgIdx],

llvm/include/llvm/ADT/STLExtras.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,11 +1291,15 @@ class indexed_accessor_range_base {
12911291
}
12921292

12931293
/// Compare this range with another.
1294-
template <typename OtherT> bool operator==(const OtherT &rhs) const {
1295-
return std::equal(begin(), end(), rhs.begin(), rhs.end());
1296-
}
1297-
template <typename OtherT> bool operator!=(const OtherT &rhs) const {
1298-
return !(*this == rhs);
1294+
template <typename OtherT>
1295+
friend bool operator==(const indexed_accessor_range_base &lhs,
1296+
const OtherT &rhs) {
1297+
return std::equal(lhs.begin(), lhs.end(), rhs.begin(), rhs.end());
1298+
}
1299+
template <typename OtherT>
1300+
friend bool operator!=(const indexed_accessor_range_base &lhs,
1301+
const OtherT &rhs) {
1302+
return !(lhs == rhs);
12991303
}
13001304

13011305
/// Return the size of this range.

0 commit comments

Comments
 (0)