[PATCH] D95754: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:2310
 
+  S.Diags.noteNumOverloadCandidatesShown(ShownOverloads);
+
----------------
aaronpuchert wrote:
> jlebar wrote:
> > aaronpuchert wrote:
> > > Why not in the following `if`? I assume we want to show a long list not necessarily once, but only if it comes with the first error?
> > My intent was to show the long list once, even if it's not the very first error.  My thought process:
> > 
> > All things being equal, it's better to show more information to the user than less.  The problem is, at some point, the amount of information we show becomes overwhelming and spammy.  Particularly problematic are multiline errors, because then you get O(nm) error lines across the whole TU.  We prevent the O(nm) overwhelm by limiting the number of lines a particular error can produce (using the mechanism in question here, or the template backtrace limit, etc), and then also limiting the total number of individual errors before we stop printing those.
> > 
> > With this change, we display the full(ish) error the first time it occurs and then the truncated error every other time.  So in total it's O(n + m) rather than O(nm).
> Got it, but currently you'd not be exhausting the option to print a long list once: when the first overload resolution error has fewer candidates than the limit you'd still say we stop printing long lists of notes from now on. That confused me because you're calling `noteNumOverloadCandidatesShown` but you might not actually have printed that note.
> 
> If you're going by the //O(n+m)// argument, you could put this call into the `if (SuppressedOverloads)` and still stay under that limit.
> currently you'd not be exhausting the option to print a long list once: when the first overload resolution error has fewer candidates than the limit you'd still say we stop printing long lists of notes from now on.

But if we print <= 4 overloads then noteNumOverloadCandidatesShown has no effect?

OTOH if we move it into the `if` then imagine a case where NumOverloads starts at 32 and we print 16 overloads.  In that case we don't suppress any overloads.  But the *next* time we still want to limit it to only 4.  So moving it into the `if` would do the wrong thing.

Unless I'm misunderstanding?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95754/new/

https://reviews.llvm.org/D95754