Unverified Commit 47747da6 authored by Utkarsh Saxena's avatar Utkarsh Saxena Committed by GitHub
Browse files

[clang] Handle templated operators with reversed arguments (#69595) 

https://github.com/llvm/llvm-project/pull/68999 correctly computed
conversion sequence for reversed args to a template operators. This was
a breaking change as code, previously accepted in C++17, starts to break
in C++20.

Example:
```cpp
struct P {};
template<class S> bool operator==(const P&, const S &);

struct A : public P {};
struct B : public P {};
bool check(A a, B b) { return a == b; }  // This is now ambiguous in C++20.
```

In order to minimise widespread breakages, as a clang extension, we had
previously accepted such ambiguities with a warning
(`-Wambiguous-reversed-operator`) for non-template operators. Due to the
same reasons, we extend this relaxation for template operators.

Fixes https://github.com/llvm/llvm-project/issues/53954
parent ba8565fb

clang/docs/ReleaseNotes.rst

+21 −0
Original line number Diff line number Diff line
@@ -37,6 +37,27 @@ These changes are ones which we think may surprise users when upgrading to 
Clang |release| because of the opportunity they pose for disruption to existing

code bases.



- Fix a bug in reversed argument for templated operators.

  This breaks code in C++20 which was previously accepted in C++17. Eg:



  .. code-block:: cpp



    struct P {};

    template<class S> bool operator==(const P&, const S&);



    struct A : public P {};

    struct B : public P {};



    // This equality is now ambiguous in C++20.

    bool ambiguous(A a, B b) { return a == b; }



    template<class S> bool operator!=(const P&, const S&);

    // Ok. Found a matching operator!=.

    bool fine(A a, B b) { return a == b; }



  To reduce such widespread breakages, as an extension, Clang accepts this code

  with an existing warning ``-Wambiguous-reversed-operator`` warning.

  Fixes `GH <https://github.com/llvm/llvm-project/issues/53954>`_.



C/C++ Language Potentially Breaking Changes

-------------------------------------------

clang/lib/Sema/SemaOverload.cpp

+18 −10
Original line number Diff line number Diff line
@@ -7688,7 +7688,7 @@ bool Sema::CheckNonDependentConversions( 
    QualType ParamType = ParamTypes[I + Offset];

    if (!ParamType->isDependentType()) {

      unsigned ConvIdx = PO == OverloadCandidateParamOrder::Reversed

                             ? 0

                             ? Args.size() - 1 - (ThisConversions + I)

                             : (ThisConversions + I);

      Conversions[ConvIdx]

        = TryCopyInitialization(*this, Args[I], ParamType,
@@ -10085,11 +10085,19 @@ getImplicitObjectParamType(ASTContext &Context, const FunctionDecl *F) { 
  return M->getFunctionObjectParameterReferenceType();

}













static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,













// As a Clang extension, allow ambiguity among F1 and F2 if they represent













// represent the same entity.













static bool allowAmbiguity(ASTContext &Context, const FunctionDecl *F1,















                           const FunctionDecl *F2) {















  if (declaresSameEntity(F1, F2))















    return true;

























  if (F1->isTemplateInstantiation() && F2->isTemplateInstantiation() &&













      declaresSameEntity(F1->getPrimaryTemplate(), F2->getPrimaryTemplate())) {













    return true;













  }













  // TODO: It is not clear whether comparing parameters is necessary (i.e.













  // different functions with same params). Consider removing this (as no test













  // fail w/o it).















  auto NextParam = [&](const FunctionDecl *F, unsigned &I, bool First) {















    if (First) {















      if (std::optional<QualType> T = getImplicitObjectParamType(Context, F))










@@ -10274,14 +10282,14 @@ bool clang::isBetterOverloadCandidate(













    case ImplicitConversionSequence::Worse:















      if (Cand1.Function && Cand2.Function &&















          Cand1.isReversed() != Cand2.isReversed() &&













          haveSameParameterTypes(S.Context, Cand1.Function, Cand2.Function)) {













          allowAmbiguity(S.Context, Cand1.Function, Cand2.Function)) {















        // Work around large-scale breakage caused by considering reversed















        // forms of operator== in C++20:















        //













        // When comparing a function against a reversed function with the same













        // parameter types, if we have a better conversion for one argument and













        // a worse conversion for the other, the implicit conversion sequences













        // are treated as being equally good.













        // When comparing a function against a reversed function, if we have a













        // better conversion for one argument and a worse conversion for the













        // other, the implicit conversion sequences are treated as being equally













        // good.















        //















        // This prevents a comparison function from being considered ambiguous















        // with a reversed form that is written in the same way.










@@ -14421,7 +14429,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,













          llvm::SmallVector<FunctionDecl*, 4> AmbiguousWith;















          for (OverloadCandidate &Cand : CandidateSet) {















            if (Cand.Viable && Cand.Function && Cand.isReversed() &&













                haveSameParameterTypes(Context, Cand.Function, FnDecl)) {













                allowAmbiguity(Context, Cand.Function, FnDecl)) {















              for (unsigned ArgIdx = 0; ArgIdx < 2; ++ArgIdx) {















                if (CompareImplicitConversionSequences(















                        *this, OpLoc, Cand.Conversions[ArgIdx],

























clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp



+61
−0






























Original line number
Diff line number
Diff line








@@ -324,6 +324,67 @@ bool x = X() == X(); // expected-warning {{ambiguous}}













}















} // namespace P2468R2





























namespace GH53954{













namespace friend_template_1 {













struct P {













    template <class T>













    friend bool operator==(const P&, const T&); // expected-note {{candidate}} \













                                                // expected-note {{ambiguous candidate function with reversed arguments}}













};













struct A : public P {};













struct B : public P {};













bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}













}



























namespace friend_template_2 {













struct P {













    template <class T>













    friend bool operator==(const T&, const P&); // expected-note {{candidate}} \













                                                // expected-note {{ambiguous candidate function with reversed arguments}}













};













struct A : public P {};













struct B : public P {};













bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}













}



























namespace member_template {













struct P {













  template<class S>













  bool operator==(const S &) const; // expected-note {{candidate}} \













                                    // expected-note {{ambiguous candidate function with reversed arguments}}













};













struct A : public P {};













struct B : public P {};













bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}













}



























namespace non_member_template_1 {













struct P {};













template<class S>













bool operator==(const P&, const S &); // expected-note {{candidate}} \













                                      // expected-note {{ambiguous candidate function with reversed arguments}}



























struct A : public P {};













struct B : public P {};













bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}



























template<class S>













bool operator!=(const P&, const S &);













bool fine(A a, B b) { return a == b; } // Ok. Found a matching operator!=.













}













}



























namespace non_member_template_2 {













struct P {};













template<class S>













bool operator==(const S&, const P&); // expected-note {{candidate}} \













                                     // expected-note {{ambiguous candidate function with reversed arguments}}



























struct A : public P {};













struct B : public P {};













bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}













}





























#else // NO_ERRORS































namespace problem_cases {