[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

Author: jlebar

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 different 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

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;

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))

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}}

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