This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfebf5c97bba7: [Sema] Change order of displayed overloads in
diagnostics (authored by ilya-biryukov).
Changed prior to commit:
https://reviews.llvm.org/D159351?vs=557839&id=557840#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159351/new/
https://reviews.llvm.org/D159351
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/overloaded-operators-display-order-crash.cpp
Index: clang/test/SemaCXX/overloaded-operators-display-order-crash.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/overloaded-operators-display-order-crash.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s
+
+namespace ambig {
+ struct Foo {
+ operator int();
+ operator const char *();
+ };
+
+
+ void func(const char*, long);
+ void func(const char*, const char*);
+ void func(int, int);
+
+ bool doit(Foo x) {
+ func(x, x); // expected-error {{call to 'func' is ambiguous}}
+ // expected-note@* 3{{candidate}}
+ // Check that two functions with best conversions are at the top.
+ // CHECK: error: call to 'func' is ambiguous
+ // CHECK-NEXT: func(x, x)
+ // CHECK-NEXT: ^~~~
+ // CHECK-NEXT: note: candidate function
+ // CHECK-NEXT: void func(const char*, const char*)
+ // CHECK-NEXT: ^
+ // CHECK-NEXT: note: candidate function
+ // CHECK-NEXT: void func(int, int)
+ }
+}
+
+namespace bad_conversion {
+ struct Foo {
+ operator int();
+ operator const char *();
+ };
+
+
+ void func(double*, const char*, long);
+ void func(double*, const char*, const char*);
+ void func(double*, int, int);
+
+ bool doit(Foo x) {
+ func((int*)0, x, x); // expected-error {{no matching function for call to 'func'}}
+ // expected-note@* 3{{candidate}}
+ // Check that two functions with best conversions are at the top.
+ // CHECK: error: no matching function for call to 'func'
+ // CHECK-NEXT: func((int*)0, x, x)
+ // CHECK-NEXT: ^~~~
+ // CHECK-NEXT: note: candidate function
+ // CHECK-NEXT: void func(double*, const char*, const char*)
+ // CHECK-NEXT: ^
+ // CHECK-NEXT: note: candidate function
+ // CHECK-NEXT: void func(double*, int, int)
+ }
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -38,8 +38,10 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
#include <algorithm>
+#include <cstddef>
#include <cstdlib>
#include <optional>
@@ -12017,6 +12019,7 @@
}
namespace {
+
struct CompareOverloadCandidatesForDisplay {
Sema &S;
SourceLocation Loc;
@@ -12054,13 +12057,9 @@
if (L->Viable) {
if (!R->Viable) return true;
- // TODO: introduce a tri-valued comparison for overload
- // candidates. Would be more worthwhile if we had a sort
- // that could exploit it.
- if (isBetterOverloadCandidate(S, *L, *R, SourceLocation(), CSK))
- return true;
- if (isBetterOverloadCandidate(S, *R, *L, SourceLocation(), CSK))
- return false;
+ if (int Ord = CompareConversions(*L, *R))
+ return Ord < 0;
+ // Use other tie breakers.
} else if (R->Viable)
return false;
@@ -12112,30 +12111,8 @@
}
// If there's any ordering between the defined conversions...
- // FIXME: this might not be transitive.
- assert(L->Conversions.size() == R->Conversions.size());
-
- int leftBetter = 0;
- unsigned I = (L->IgnoreObjectArgument || R->IgnoreObjectArgument);
- for (unsigned E = L->Conversions.size(); I != E; ++I) {
- switch (CompareImplicitConversionSequences(S, Loc,
- L->Conversions[I],
- R->Conversions[I])) {
- case ImplicitConversionSequence::Better:
- leftBetter++;
- break;
-
- case ImplicitConversionSequence::Worse:
- leftBetter--;
- break;
-
- case ImplicitConversionSequence::Indistinguishable:
- break;
- }
- }
- if (leftBetter > 0) return true;
- if (leftBetter < 0) return false;
-
+ if (int Ord = CompareConversions(*L, *R))
+ return Ord < 0;
} else if (RFailureKind == ovl_fail_bad_conversion)
return false;
@@ -12157,10 +12134,66 @@
SourceLocation RLoc = GetLocationForCandidate(R);
// Put candidates without locations (e.g. builtins) at the end.
- if (LLoc.isInvalid()) return false;
- if (RLoc.isInvalid()) return true;
+ if (LLoc.isValid() && RLoc.isValid())
+ return S.SourceMgr.isBeforeInTranslationUnit(LLoc, RLoc);
+ if (LLoc.isValid() && !RLoc.isValid())
+ return true;
+ if (RLoc.isValid() && !LLoc.isValid())
+ return false;
+ assert(!LLoc.isValid() && !RLoc.isValid());
+ // For builtins and other functions without locations, fallback to the order
+ // in which they were added into the candidate set.
+ return L < R;
+ }
- return S.SourceMgr.isBeforeInTranslationUnit(LLoc, RLoc);
+private:
+ struct ConversionSignals {
+ unsigned KindRank = 0;
+ ImplicitConversionRank Rank = ICR_Exact_Match;
+
+ static ConversionSignals ForSequence(ImplicitConversionSequence &Seq) {
+ ConversionSignals Sig;
+ Sig.KindRank = Seq.getKindRank();
+ if (Seq.isStandard())
+ Sig.Rank = Seq.Standard.getRank();
+ else if (Seq.isUserDefined())
+ Sig.Rank = Seq.UserDefined.After.getRank();
+ // We intend StaticObjectArgumentConversion to compare the same as
+ // StandardConversion with ICR_ExactMatch rank.
+ return Sig;
+ }
+
+ static ConversionSignals ForObjectArgument() {
+ // We intend StaticObjectArgumentConversion to compare the same as
+ // StandardConversion with ICR_ExactMatch rank. Default give us that.
+ return {};
+ }
+ };
+
+ // Returns -1 if conversions in L are considered better.
+ // 0 if they are considered indistinguishable.
+ // 1 if conversions in R are better.
+ int CompareConversions(const OverloadCandidate &L,
+ const OverloadCandidate &R) {
+ // We cannot use `isBetterOverloadCandidate` because it is defined
+ // according to the C++ standard and provides a partial order, but we need
+ // a total order as this function is used in sort.
+ assert(L.Conversions.size() == R.Conversions.size());
+ for (unsigned I = 0, N = L.Conversions.size(); I != N; ++I) {
+ auto LS = L.IgnoreObjectArgument && I == 0
+ ? ConversionSignals::ForObjectArgument()
+ : ConversionSignals::ForSequence(L.Conversions[I]);
+ auto RS = R.IgnoreObjectArgument
+ ? ConversionSignals::ForObjectArgument()
+ : ConversionSignals::ForSequence(R.Conversions[I]);
+ if (std::tie(LS.KindRank, LS.Rank) != std::tie(RS.KindRank, RS.Rank))
+ return std::tie(LS.KindRank, LS.Rank) < std::tie(RS.KindRank, RS.Rank)
+ ? -1
+ : 1;
+ }
+ // FIXME: find a way to compare templates for being more or less
+ // specialized that provides a strict weak ordering.
+ return 0;
}
};
}
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -356,6 +356,30 @@
nonportable construct that has different semantics from a constant-sized
array. (`#62836 <https://github.com/llvm/llvm-project/issues/62836>`_)
+- Clang changed the order in which it displays candidate functions on overloading failures.
+ Previously, Clang used definition of ordering from the C++ Standard. The order defined in
+ the Standard is partial and is not suited for sorting. Instead, Clang now uses a strict
+ order that still attempts to push more relevant functions to the top by comparing their
+ corresponding conversions. In some cases, this results in better order. E.g., for the
+ following code
+
+ .. code-block:: cpp
+ struct Foo {
+ operator int();
+ operator const char*();
+ };
+
+ void test() { Foo() - Foo(); }
+
+ Clang now produces a list with two most relevant builtin operators at the top,
+ i.e. ``operator-(int, int)`` and ``operator-(const char*, const char*)``.
+ Previously ``operator-(const char*, const char*)`` was the first element,
+ but ``operator-(int, int)`` was only the 13th element in the output.
+ However, new implementation does not take into account some aspects of
+ C++ semantics, e.g. which function template is more specialized. This
+ can sometimes lead to worse ordering.
+
+
Bug Fixes in This Version
-------------------------
- Fixed an issue where a class template specialization whose declaration is
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
