[clang] c90dac2 - [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.
Justin Lebar via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 25 17:47:08 PST 2021
Author: Justin Lebar
Date: 2021-02-25T17:45:19-08:00
New Revision: c90dac27e94ec354a3e8919556ac5bc89b62c731
URL: https://github.com/llvm/llvm-project/commit/c90dac27e94ec354a3e8919556ac5bc89b62c731
DIFF: https://github.com/llvm/llvm-project/commit/c90dac27e94ec354a3e8919556ac5bc89b62c731.diff
LOG: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.
Previously, -fshow-overloads=best always showed 4 candidates. The
problem is, when this isn't enough, you're kind of up a creek; the only
option available is to recompile with different flags. This can be
quite expensive!
With this change, we try to strike a compromise. The *first* error with
more than 4 candidates will show up to 32 candidates. All further
errors continue to show only 4 candidates.
The hope is that this way, users will have *some chance* of making
forward progress, without facing unbounded amounts of error spam.
Differential Revision: https://reviews.llvm.org/D95754
Added:
Modified:
clang/include/clang/Basic/Diagnostic.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
clang/test/SemaCXX/overloaded-builtin-operators.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index a3cc3af5a74a..8f94d271aaa4 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -273,6 +273,13 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
// Which overload candidates to show.
OverloadsShown ShowOverloads = Ovl_All;
+ // With Ovl_Best, the number of overload candidates to show when we encounter
+ // an error.
+ //
+ // The value here is the number of candidates to show in the first nontrivial
+ // error. Future errors may show a
diff erent number of candidates.
+ unsigned NumOverloadsToShow = 32;
+
// Cap of # errors emitted, 0 -> no limit.
unsigned ErrorLimit = 0;
@@ -707,6 +714,36 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
}
OverloadsShown getShowOverloads() const { return ShowOverloads; }
+ /// When a call or operator fails, print out up to this many candidate
+ /// overloads as suggestions.
+ ///
+ /// With Ovl_Best, we set a high limit for the first nontrivial overload set
+ /// we print, and a lower limit for later sets. This way the user has a
+ /// chance of diagnosing at least one callsite in their program without
+ /// having to recompile with -fshow-overloads=all.
+ unsigned getNumOverloadCandidatesToShow() const {
+ switch (getShowOverloads()) {
+ case Ovl_All:
+ // INT_MAX rather than UINT_MAX so that we don't have to think about the
+ // effect of implicit conversions on this value. In practice we'll never
+ // hit 2^31 candidates anyway.
+ return std::numeric_limits<int>::max();
+ case Ovl_Best:
+ return NumOverloadsToShow;
+ }
+ }
+
+ /// Call this after showing N overload candidates. This influences the value
+ /// returned by later calls to getNumOverloadCandidatesToShow().
+ void overloadCandidatesShown(unsigned N) {
+ // Current heuristic: Start out with a large value for NumOverloadsToShow,
+ // and then once we print one nontrivially-large overload set, decrease it
+ // for future calls.
+ if (N > 4) {
+ NumOverloadsToShow = 4;
+ }
+ }
+
/// Pretend that the last diagnostic issued was ignored, so any
/// subsequent notes will be suppressed, or restore a prior ignoring
/// state after ignoring some diagnostics and their notes, possibly in
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 46c553afdb62..9885c1fadced 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2312,9 +2312,7 @@ static void noteOverloads(Sema &S, const UnresolvedSetImpl &Overloads,
int SuppressedOverloads = 0;
for (UnresolvedSetImpl::iterator It = Overloads.begin(),
DeclsEnd = Overloads.end(); It != DeclsEnd; ++It) {
- // FIXME: Magic number for max shown overloads stolen from
- // OverloadCandidateSet::NoteCandidates.
- if (ShownOverloads >= 4 && S.Diags.getShowOverloads() == Ovl_Best) {
+ if (ShownOverloads >= S.Diags.getNumOverloadCandidatesToShow()) {
++SuppressedOverloads;
continue;
}
@@ -2330,6 +2328,8 @@ static void noteOverloads(Sema &S, const UnresolvedSetImpl &Overloads,
++ShownOverloads;
}
+ S.Diags.overloadCandidatesShown(ShownOverloads);
+
if (SuppressedOverloads)
S.Diag(FinalNoteLoc, diag::note_ovl_too_many_candidates)
<< SuppressedOverloads;
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7fe7466725fa..434f5dfb442c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10355,18 +10355,15 @@ void ImplicitConversionSequence::DiagnoseAmbiguousConversion(
const PartialDiagnostic &PDiag) const {
S.Diag(CaretLoc, PDiag)
<< Ambiguous.getFromType() << Ambiguous.getToType();
- // FIXME: The note limiting machinery is borrowed from
- // OverloadCandidateSet::NoteCandidates; there's an opportunity for
- // refactoring here.
- const OverloadsShown ShowOverloads = S.Diags.getShowOverloads();
unsigned CandsShown = 0;
AmbiguousConversionSequence::const_iterator I, E;
for (I = Ambiguous.begin(), E = Ambiguous.end(); I != E; ++I) {
- if (CandsShown >= 4 && ShowOverloads == Ovl_Best)
+ if (CandsShown >= S.Diags.getNumOverloadCandidatesToShow())
break;
++CandsShown;
S.NoteOverloadCandidate(I->first, I->second);
}
+ S.Diags.overloadCandidatesShown(CandsShown);
if (I != E)
S.Diag(SourceLocation(), diag::note_ovl_too_many_candidates) << int(E - I);
}
@@ -11644,7 +11641,7 @@ bool OverloadCandidateSet::shouldDeferDiags(Sema &S, ArrayRef<Expr *> Args,
(Cand.Function->template hasAttr<CUDAHostAttr>() &&
Cand.Function->template hasAttr<CUDADeviceAttr>());
});
- DeferHint = WrongSidedCands.size();
+ DeferHint = !WrongSidedCands.empty();
}
return DeferHint;
}
@@ -11677,10 +11674,8 @@ void OverloadCandidateSet::NoteCandidates(Sema &S, ArrayRef<Expr *> Args,
for (; I != E; ++I) {
OverloadCandidate *Cand = *I;
- // Set an arbitrary limit on the number of candidate functions we'll spam
- // the user with. FIXME: This limit should depend on details of the
- // candidate list.
- if (CandsShown >= 4 && ShowOverloads == Ovl_Best) {
+ if (CandsShown >= S.Diags.getNumOverloadCandidatesToShow() &&
+ ShowOverloads == Ovl_Best) {
break;
}
++CandsShown;
@@ -11709,6 +11704,10 @@ void OverloadCandidateSet::NoteCandidates(Sema &S, ArrayRef<Expr *> Args,
}
}
+ // Inform S.Diags that we've shown an overload set with N elements. This may
+ // inform the future value of S.Diags.getNumOverloadCandidatesToShow().
+ S.Diags.overloadCandidatesShown(CandsShown);
+
if (I != E)
S.Diag(OpLoc, diag::note_ovl_too_many_candidates,
shouldDeferDiags(S, Args, OpLoc))
diff --git a/clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp b/clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
index 5cd26fc31aef..21b752f696fc 100644
--- a/clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
+++ b/clang/test/SemaCXX/ambiguous-conversion-show-overload.cpp
@@ -10,9 +10,20 @@ struct S {
S(signed int*);
};
void f(const S& s);
-void g() {
- f(0);
-}
+
+// First call to f emits all candidates. Second call emits just the first 4.
+void g() { f(0); }
+// CHECK: {{conversion from 'int' to 'const S' is ambiguous}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+// CHECK-NEXT: {{candidate constructor}}
+
+void h() { f(0); }
// CHECK: {{conversion from 'int' to 'const S' is ambiguous}}
// CHECK-NEXT: {{candidate constructor}}
// CHECK-NEXT: {{candidate constructor}}
diff --git a/clang/test/SemaCXX/overloaded-builtin-operators.cpp b/clang/test/SemaCXX/overloaded-builtin-operators.cpp
index d237dfe6c091..0c76df79e6e1 100644
--- a/clang/test/SemaCXX/overloaded-builtin-operators.cpp
+++ b/clang/test/SemaCXX/overloaded-builtin-operators.cpp
@@ -195,8 +195,7 @@ struct A {
void test_dr425(A a) {
(void)(1.0f * a); // expected-error{{ambiguous}} \
- // expected-note 4{{candidate}} \
- // expected-note {{remaining 8 candidates omitted; pass -fshow-overloads=all to show them}}
+ // expected-note 12{{candidate}}
}
// pr5432
More information about the cfe-commits
mailing list