aaronpuchert added inline comments.
================ Comment at: clang/lib/Sema/Sema.cpp:2310 + S.Diags.noteNumOverloadCandidatesShown(ShownOverloads); + ---------------- jlebar wrote: > 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? You're absolutely right. I guess I was confused by the function name `noteNumOverloadCandidatesShown`, thinking that calling it would indicate that we've shown the note below. Perhaps we can just drop the `note` or find another verb? Or you could explain how the name is intended to be read. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95754/new/ https://reviews.llvm.org/D95754 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
